Review (NS-154): Wayback indexer component

Author

Colin

Moderator

Colin

State

Closed

Objectives

This component creates an unsorted cdx file for each file in the archive.

Total Time Used (Coding,Documentation,Review): 

time used jolf - 1md
csr 16md

General comments:

Description

Classification

Status

Some of the logging levels are too low. The timer thread should log at INFO each time it runs and we should consider adding INFO level logging for each file indexed.

Minor

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/GenericHibernateDAO.java', revision 1407

Lines

Description

Classification

Status

36-37

finish javadoc

Cosmetic

NOTOK

41

javadoc

Cosmetic

NOTOK

51

javadoc

Cosmetic

NOTOK

60

javadoc

Cosmetic

NOTOK

67

javadoc

Cosmetic

NOTOK

75

javadoc

Cosmetic

NOTOK

91

javadoc

Cosmetic

NOTOK

109-111

unnecessary empty lines

Cosmetic

NOTOK

Comments on file 'trunk/build.xml', revision 1391

Lines

Description

Classification

Status

123-136

Get Søren to accept these new external libraries.

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/WaybackIndexer.java', revision 1407

Lines

Description

Classification

Status

74

javadoc

Cosmetic

NOTOK

85

javadoc

Cosmetic

NOTOK

106

javadoc

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/ArchiveFile.java', revision 1407

Lines

Description

Classification

Status

93

javadoc

Cosmetic

NOTOK

98

javadoc

Cosmetic

NOTOK

102

javadoc

Cosmetic

NOTOK

106

javadoc

Cosmetic

NOTOK

110

javadoc

Cosmetic

NOTOK

114

javadoc

Cosmetic

NOTOK

118

javadoc

Cosmetic

NOTOK

127

What does that mean?

NA

NOTOK

132

javadoc

Cosmetic

NOTOK

136

javadoc

Cosmetic

NOTOK

140

javadoc

Cosmetic

NOTOK

144

javadoc

Cosmetic

NOTOK

148

javadoc

Cosmetic

NOTOK

182-188

comment about what is done here! why only one file?

Cosmetic

NOTOK

191

javadoc

Cosmetic

NOTOK

192-209

Please use some empty lines and comments in this function. It is currently quite difficult to read.

Cosmetic

NOTOK

212

javadoc

Cosmetic

NOTOK

230

javadoc

Cosmetic

NOTOK

267

javadoc

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/HibernateUtil.java', revision 1407

Lines

Description

Classification

Status

68-104

All property names should be contants.

Cosmetic

NOTOK

98-105

Can the settings be empty? What would happen? And why is this the only settings, which are tested for whether they are empty? Write comment about it!

Cosmetic

NOTOK

117

javadoc

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/ArchiveFileDAO.java', revision 1407

Lines

Description

Classification

Status

37

javadoc

Cosmetic

NOTOK

62

odd indentation

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/NetarchiveResourceStore.java', revision 1391

Lines

Description

Classification

Status

216

javadoc

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/IndexerQueue.java', revision 1407

Lines

Description

Classification

Status

67-68

Javadoc

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/WaybackIndexerApplication.java', revision 1407

Lines

Description

Classification

Status

26

javadoc class

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/FileNameHarvester.java', revision 1407

Lines

Description

Classification

Status

65-66

no need to create a variable for the instance of the batchjob. Just: client.batch(new FileListJob(), ...

Cosmetic

NOTOK

74-81

What is done if the line already exists? nothing. Do something, or at least make a comment about it.

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/wayback/WaybackSettings.java', revision 1407

Lines

Description

Classification

Status

General

This only contains sporadic javadoc, which deviates from our code-style.

Cosmetic

NOTOK