Review (NS-27): Review of changes to batch framework

Author

Søren

Moderator

Søren

State

Closed

Description

Classification

Status

Lines

Description

Classification

Status

125

ArgumentNotValid.checkNotNullOrEmpty(appID); ?

Cosmetic

OK

131

"from" => "from bitarchive"

Cosmetic

OK

221-229

Does not check ArgumentNotValid on: 'noOfFilesProcessed', 'filesFailed', 'remoteFile', 'errMsg', 'exceptions'.

Cosmetic

OK

305, 314

Perhaps consider to change to 'private' with 'get' and 'set' methods instead of public local variables.

Cosmetic

POSTPONED

314

Error messages is appended by 'String' + 'String'. Perhaps change to StringBuilder?

Cosmetic

POSTPONED

354-355

Log this

Cosmetic

OK

370

Check for ArgumentNotValid?

Cosmetic

REJECTED

412-421

'found' only used once. Thus change to if(!missingRespondents.remove(bitarchiveID)) { ... } OR write comment about 'found'

Cosmetic

OK

522

check for ArgumentNotValid?

Cosmetic

OK

Lines

Description

Classification

Status

149, 152

Add constants for ".class", and ".jar", and maybe "."

Cosmetic

NOTOK

164

Spell CWD out: Current working directory

Cosmetic

NOTOK

202, 204

public or private variables?

Cosmetic

NOTOK

205-206

What is meant by this comment? Add TODO: Use the HelpFormatter class to print out Usage information

Cosmetic

NOTOK

215, 217, 220, 224, 227, 234, 237

Consider replacing by constants

Cosmetic

NOTOK

220-221

Indentation

Cosmetic

NOTOK

222

Odd place for an empty line.

Cosmetic

NOTOK

241

Javadoc. public/private/protected not defined for this method?

Cosmetic

NOTOK

252

Javadoc. public/private/protected not defined for this method?

Cosmetic

NOTOK

266-267

Two empty lines in a row.

Cosmetic

NOTOK

268-269

Shouldn't variables be placed next to each other, and not randomly in the middle of the methods?

Cosmetic

NOTOK

340

change "," to a constant.

Cosmetic

NOTOK

355

Make into bug or feature request (if not already) and refer to this. Or implement

Cosmetic

NOTOK

505-506

Two empty lines in a row

Cosmetic

NOTOK

Lines

Description

Classification

Status

211-213

Identical messages -> make into string before use.

Cosmetic

OK

268-271

ArgumentNotValid.checkTrue(file.isFile(), "....");

Cosmetic

OK

273

Better to use StringBuilder.

Cosmetic

OK

387, 407

Examine the replyMsg, and check if it is ok. If it is not ok, log a message at level WARN

Cosmetic

OK

412, 426

Examine the replyMsg, and check if it is ok. If it is not ok, log a message at level WARN

Cosmetic

OK

Lines

Description

Classification

Status

51

".*" should perhaps be a constants (static final) variable.

Cosmetic

NOTOK

109

".*" as a constant (static final) variable (same constant as previous)

Cosmetic

NOTOK

138

Make an OR_SEPARATOR constant for "|"

Cosmetic

NOTOK

164, 167

It does not need to be ARC-files we're processing

Cosmetic

NOTOK

174-175, 177-178

It does not need to be ARC-files we're processing

Cosmetic

NOTOK

206, 222

Log the case, if maxExceptions have been reached

Cosmetic

NOTOK

235

Log the case, if maxExceptions have been reached

Cosmetic

NOTOK

259

Add a TODO, that this should/could be a setting

Cosmetic

NOTOK

330

Replace -1 with UNKNOWN_OFFSET

Cosmetic

NOTOK

393-398

Javadoc this method.

Cosmetic

NOTOK

Lines

Description

Classification

Status

General

Rename BatchFilter as ARCBatchFilter.

Cosmetic

NOTOK

117

Make comment about 'mimetypeIsOk' checks for ArgumentNotValid, or check the mimetype for ArgumentNotValid.

Cosmetic

NOTOK

Lines

Description

Classification

Status

64

Add local variables for the 'null', and '0' constants: String name = null; int offset = 0;

Cosmetic

NOTOK

Lines

Description

Classification

Status

43

javadoc

Cosmetic

NOTOK

49

Replace "used for batching" by "processed by the batch job"

Cosmetic

NOTOK

55-58

This would be the same as: ArgumentNotValid.checkNotNull(incomingFile[i], "Null element at index " + i + " in file lise for batch."); Thus using the check function to check for NotNull.

Cosmetic

NOTOK

84

Should we add the initialization exception to the list of exceptions for job? Add as TODO

Cosmetic

NOTOK

90

Should we add this finish exception to the list of exceptions for job? Add as TODO

Cosmetic

NOTOK

108-109

Should we add this exception thrown during processing to the list of exceptions for job? Add as TODO

Cosmetic

NOTOK

Lines

Description

Classification

Status

48

language: "resent" i.e. forwarded

Cosmetic

NOTOK

74

Rename con as jmscon

Cosmetic

NOTOK

120

Consider checking that inbMsg is not null

Cosmetic

NOTOK

144

Consider checking that beMsg is not null

Cosmetic

NOTOK

176

Consider checking that hbMsg is not null

Cosmetic

NOTOK

192

'o' is not a very good variable name og an observable object. Use 'obs' or equivilent;

Cosmetic

NOTOK

202, 206, 212

Add respectively "an", "an", and "a" after "Received"

Cosmetic

NOTOK

Lines

Description

Classification

Status

45

"to identifier for" => "to identify"

Cosmetic

NOTOK

48

Rename "BA_ApplicationId" as "BitarchiveApplicationId"

Cosmetic

NOTOK

59-61

three empty lines in a row. Delete two of them.

Cosmetic

NOTOK

73

rename to BitarchiveApplicationId

Cosmetic

NOTOK

76

"he remote file" => "The remote file"

Cosmetic

NOTOK

177

Add ArgumentNotValid check

Cosmetic

NOTOK

Lines

Description

Classification

Status

116

Add a JAVA_PACKAGE_SEPARATOR constant for "." Add a DIRECTORY_SEPARATOR constant for "/"

Cosmetic

NOTOK

119

Replace "No data loaded!!!!!" with "No data loaded for class with name '" + className + "'."

Cosmetic

NOTOK

157

Check argument os

Cosmetic

NOTOK

170

'Cannout' = spelling error.

Cosmetic

NOTOK

192

"Finish up" => "Finish"

Cosmetic

NOTOK

Lines

Description

Classification

Status

35

Delete line

Cosmetic

NOTOK

58-59

".., that run this batchJob" => ".., that ran this batch job"

Cosmetic

NOTOK

66-70

Check for ArgumentNotValid.

Cosmetic

NOTOK

90

Make "ALL_BITARCHIVE_APPS" a constant

Cosmetic

NOTOK

160

".. to copy into '" => ".. to copy into target file '"

Cosmetic

NOTOK

161

"batch job on" => "batch job run on "

Cosmetic

NOTOK

168

"so this" => so this method

Cosmetic

NOTOK

187

the variable 'stream' cannot be printed out. Just write "append to output stream".

Cosmetic

NOTOK

188

"batch job on" => batch job run on bitarchive application

Cosmetic

NOTOK

Lines

Description

Classification

Status

57

Check args arcfile and cdxstream

Cosmetic

NOTOK

59

Remove superfluous whitespace

Cosmetic

NOTOK

75

Remove superfluous whitespace

Cosmetic

NOTOK

101-105

replace with: ArgumentNotValid.checkTrue(arcFileDirectory.isDirectory() && arcFileDirectory.canRead(), "...");

Cosmetic

NOTOK

106-110

replace with: ArgumentNotValid.checkTrue(cdxFileDirectory.isDirectory() && cdxFileDirectory.canRead(), "...");

Cosmetic

NOTOK

140

could be written if (!exceptions.isEmpty())

Cosmetic

NOTOK

Lines

Description

Classification

Status

33-36, 71-73

Only three of the four new values are included. Add the fourth

Cosmetic

NOTOK

Lines

Description

Classification

Status

106

ArgumentNotValid.checkNotNegative on 'index' ?

Cosmetic

NOTOK

112

don't use 'arcfile' before it is checked for 'ArgumentNotValid' on the following line.

Cosmetic

NOTOK

136-140

Remove uncommented code

Cosmetic

NOTOK

183

check for ArgumentNotValid before use of argument-variables.

Cosmetic

NOTOK

261-264

Identical messages, use a string.

Cosmetic

NOTOK

318

Move log after argument validation

Cosmetic

NOTOK

338

Should this not be synchronized to avoid multiple callers beginning the construction of a new Bitarchive object

Cosmetic

NOTOK

Lines

Description

Classification

Status

52, 56, 58-60

Javadoc.

Cosmetic

NOTOK

76

Add ArgumentNotValid check for file, and file existence

Cosmetic

NOTOK

90

make ArgumentNotValid checks for arcfile (Not Null Not empty), and index (not negative)

Cosmetic

NOTOK

123

Note that this argument is not used in this implementation

Cosmetic

NOTOK

128

check for Argument not valid?

Cosmetic

NOTOK

133

Only need to write "batch" once

Cosmetic

NOTOK

143

Check for ArgumentNotValid.

Cosmetic

NOTOK

147-148

Remove uncommented code

Cosmetic

NOTOK

163

Write "batch job" instead of just "batch"

Cosmetic

NOTOK

188-190

Throw an NotImplementedException

Cosmetic

NOTOK

199-200

Throw an NotImplementedException

Cosmetic

NOTOK

213-214

check for ArgumentNotValid.

Cosmetic

NOTOK

215

note in the javadoc, that these variables are not used

Cosmetic

NOTOK

Lines

Description

Classification

Status

52

Javadoc.

Cosmetic

NOTOK

164, 343

use instead files.toArray(new File[files.size])

Cosmetic

NOTOK

164

Too long line

Cosmetic

NOTOK

262

Add meaning that the given File object exists, is a directory, and is writable by this process.

Cosmetic

NOTOK

298-305

The 'j' is not used explicit -> change to "for (File file : filesHere)" and use 'file' instead of 'filesHere[j]'.

Cosmetic

NOTOK

300

replace 'filesHere[j]' with "filesHere[j].getAbsolutePath()"?

Cosmetic

NOTOK

319

Validate regexp as Not null and Not empty String

Cosmetic

NOTOK

329-330

Replace "Non-file" with "Not a regular File"

Cosmetic

NOTOK

364

"Corrupt" => "Possibly corrupt"

Cosmetic

NOTOK

370

Consider logging at trace

Cosmetic

NOTOK

388-395

use for (File file : files) { ... }

Cosmetic

NOTOK

404-412

use for (File file : files) { ... }

Cosmetic

NOTOK

421-429

use for (File file : files) { ... }

Cosmetic

NOTOK

471-474

Different messages.

Cosmetic

NOTOK

479-480

Why use two line for this?

Cosmetic

NOTOK

Lines

Description

Classification

Status

General

Should this also read the MINSPACELEFT setting used by the Bitarchives? Add as TODO

Cosmetic

NOTOK

71-72

Javadoc.

Cosmetic

NOTOK

79

Add a log.info message, that the DEFAULT_DIR_NAME is included as one of the fileDirs

Cosmetic

NOTOK

105

throws also IllegalState. Either add IllegalState, or remove the others.

Cosmetic

NOTOK

175

Mention that the location argument is unused in this implementation

Cosmetic

NOTOK

225

Perhaps also check for '!filesInDir.isEmpty' to aviod making a unnecessary call to the 'files.addAll' method.

Cosmetic

NOTOK

248

Remove uncommented code

Cosmetic

NOTOK

260-262

Throw NotImplementedException, and add note about this to javadoc

Cosmetic

NOTOK

271-272

Throw NotImplementedException and add note about this to javadoc

Cosmetic

NOTOK

279-280

Add that bitarchiveName is unused in this implementation of ArcRepositoryClient And is therefore not checked

Cosmetic

NOTOK

Lines

Description

Classification

Status

45

remove empty line

Cosmetic

NOTOK

97

"if file" => if file was

Cosmetic

NOTOK

135-140

This is catched by the outer try-catch, and handled exactly the same way. Remove try-catch

Minor

NOTOK

139

Add comment: "This aborts the processing of this ARC-file"

Cosmetic

NOTOK

160

log here: Unexpected exception caught. The processing of the rest of the arcrecords aborted

Cosmetic

NOTOK

193-195

Javadoc this, and argument not valid (or describe why not).

Cosmetic

NOTOK

206, 209

Are we supposed to write the 'throws ArgumentNotValid' for all the methods, which makes the ArgumentNotValid check? NO Because then it is needed in a lot of methods.

Cosmetic

NOTOK

208

Check 'ArgumentNotValid' on 'arcfile' (checkNotNull) and 'index' (checkNotNegative).

Cosmetic

NOTOK

236-238

Javadoc

Cosmetic

NOTOK

Lines

Description

Classification

Status

42

Javadoc.

Cosmetic

NOTOK