[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
[stellation-res] Code Audit for Workspace package
|
Finally I have the document for the workspace package code audit. The
detailed comments are attached in a separate file but a couple of general
comments on principles may be in order and should help explain why I
identified certain practices as problems.
The first point concerns Javadoc. Unfortunately it is very easy to let
Javadoc get out of sync. with the code. equally unfortunately when he
Javadoc gets out of sync with the code base it becomes untrustworthy and
hence essentially useless. Hence, I have identified every place in the
workspace package where I noticed discrepencies between the Javadoc and the
code. There may well be more than I spotted.
The second point concerns the use of literal constants. While these are very
commononly used they represent bugs-in-waiting, especially with open source
projects. There are two major problems with them:
a) They become a maintenance nightmare whenever it is necessary to change
the value of a literal. This is made worse where the same literal may be
used for several purposes. Does the value 0 here mean the same as the value
0 there for example.
b) It is not possible to use browsers to find where those literals that have
meaning in a specific context are used.
<start rant>
My own policy has been to never use literal values for anything except
things like strings in debug and log messages. Everything else is made a
static constant in the class where the literals are relevant, or
occasionally in classes of their own.
<end of rant/>
In any case I hope that this audit will be useful to the Stellation project.
By the way I would be quite willing to help out with any or all of the
changes that are identified in the audit. If this transpires I would proceed
as fOllows:
1. Update the Javadocs and have the changes checked in. This will make the
Javadoc for this package useful again and will have no effect on the runtime
behaviour of Stellation.
2. Get the script and Junit tests going under Windows. This may involve
fixing some of the problems identified in the audit since a number of them
are Windows-centric. If thois is the case, unit tests will also be created
to ensure that the fixes continue to be tested. I think this should be done
before any other changes are made to the code base because I cannot be fully
useful to the project until Stellation works reliably under Windows.
3. Do an edit pass to change all identified literals into static constants.
With luck this will have no negative impact and in any case the combination
of script tests and unit tests should pick up any potential errors.
4. Fix the other identified problems and write appropriate unit tests.
Each stage in this process would involve a separate code checkin.
Regards
Jonathan Gossage
Personal Email
jgossage@xxxxxxxx
Business Email
jonathan@xxxxxxxxxxxxxx
This file contains a code audit for the workspace package.
org.eclipse.stellation.workspace.Backup
1. The calling sequence for the "Backup" constructor has changed and the Javadocs should be updated.
2. The calling sequence for the "command" method has changed and the Javadoc should be updated to refer to the Javadoc for the base implementation in workspace.command.
3. The calling sequene for the two "save" methods have changed and the Javadoc needs updating.
org.eclipse.stellation.workspace.Change
1. The calling sequence for the constructor has changed and the Javadoc need updating.
2. Should the literal strings not be coded as static constant Strings to ensure that spelling and usage is consistent.
3. To follow the convention that a method is documented in the base class, theJavadoc for the "command" method should be changed to use @see to the base documentation in workspace.Command.
4. There is no Javadoc for the "batchCommand" method.
5. The "endCommand" method needs Javadoc.
6. The calling sequence for the "add()" method has changed and the Javadoc needs updating.
7. The calling sequence for the "add(String)" method has change and the Javadoc needs upgrading since the argument name has changed.
All the string constants in this method are actually keys or identifiers and should be made symbolic.
Should use the default platform directory separator instead of a hard coded '/'.
8. The method "addParent" needs to have Javadoc updated. The parameter name has changed.
9. The method "clean" needs a Javadoc entry.
10. The method "execMove" needs a Javadoc entry.
System exit codes should be made symbolic.
11. The method "execRemove" needs a Javadoc entry.
System exit codes should be made symbolic.
12. The method "fileNotFound" should log through log4J. If the user needs this output on sydout than use the appropriate appender. Logging directly to stdout is problamatic since stdout will not be usefully available in all environments (such as within Eclipse).
13. The method "inform(String") needs a Javadoc entry.
Is stdout an appropriate destination if this method invoked from within Eclipse? Would an optional callback mechanism make sense?
14. The Javadoc for the "mv" method needs updating. There are no longer any parameters.
Should use symbolic exit code values.
15. The method "mvToDirectory" needs a Javadoc update. Parameters have changed.
After discovering and reporting that the file does not exist in the workspace should either an exception be thrown or a silent exit taken. It does not make sense to proceed.
16.The method "newFileID" Javadoc entry should be updated with a @throws spec.
17. The method "rename" needs a Javadoc update. Parameter names have changed.
18. The method "rm()" should return symbolic exit codes.
It should also return the exit code passed back from "rm(String)".
19. The method "rm(String) should return symbolic exit codes.
20. The method "synchronize" should use symbolic return codes. It should also use
the return codes from the lower level "rm" and "add" calls. At the least it should accurately return the overall status of the operation.
Should the synchronize operation terminate if errors are encountered? If so it should rollback any changes and I don't see that the current environment has the bookeeping to do this.
21. The method "synchronizeDirectory" needs a Javadoc update. The parameter name has changed.
This method does not honour return codes from lower level methods.Should it do so and should a rollback mechanism be implemented?
This method should use symbolic exit codes.
org.eclipse.stellation.workspace.ChangeRequest
1. All methods need Javadoc entries.
org.eclipse.stellation.workspace.Checkin
1. Constructor "Checkin(Context)" needs Javadoc update. Parameter names have changed.
2. Method "command" should use symbolic exit codes.
It should also use symboloc names for options and other assorted strings which are really identifiers or keys and should be symbolic.
3. The method "markExecutables" is now a nop and has only one invocation from in this class. Should this both be removed?
4. The method "renumber" needs a Javadoc update. It needs a @throw spec.
org.eclipse.stellation.workspace.Checkout
1. The constructor "Context" needs a Javadoc update. A parameter name has changed.
2. Method "command" should use symbolic exit codes.
It should also use symboloc names for options and other assorted strings which are really identifiers or keys and should be symbolic.
Is this method guaranteed to be only called from the command line? Otherwise the direct print calls to stdout are inappropriate.
The call to the Unix utility "sleep" seems a bit heavy-weight when you can easily use the Java Object.Wait(time) calls. Incidentally Windows has the same problem with file modified time, so this should not be Unix specific anyway.
org.eclipse.stellation.workspace.Command
No comments for this class.
org.eclipse.stellation.workspace.Configuration
1. The constructor "Configuration(StringMap, Configuration)" needs a Javadoc update. A parameter has been added.
2. In method "addSuffix" the Javadoc @param entries should be reordered to reflect the order of declaration of the parameters.
3. In method "addTypes" the Javadoc parameters have a spelling error in "patterm".
4. In method "build" tag names etc. should all be made symbolic to facilitate maintenance.
5. In method "defaultSuffixes" there are a number of common Windows suffixes that should be added, including the following:
The following should be data types
"exe", "sys", "lib"
The following should be ignore type
"obj"
The following should be text type
"cxx", "hpp", "bat", "bas", "xml" (maybe, not sure of the implications of embedded CDATA)
"txt", "def"
6. The method "defaultTypes" should use symbolic names for default pattern names.
These names are used elsewhere.
7. In the method "getSuffixType" the Javadoc needs updating with @param spec.
8. In the method "getType" the Javadoc needs updating for the extra parameter "relativePath".
In addtion file type names should be symbolic.
9. The Javadoc comment for the method "getTypeMatch" is misleading and should be revised. It is also incomplete (needs @throw and @param specs.
10. The method "makeDefault" needs updated Javadoc with @return spec.
11. The method "output" should use symbolic definitions for tags etc.
org.eclipse.stellation.workspace.Configure
1. The constructor "Configure" needs the Javadoc updated for a parameter name change.
2. In the method "command" option names should be made symbolic.
Also configure command names should be made symbolic.
3. In method database Javadoc should be upgraded with all tags.
Also exit codes should be symbolic.
Also should use symbolic names for options.
Can it be guaranteed that this method will always be called in a command line context? Otherwise the direct output to stderr may be inappropriate.
4. Method "getCurrentConfiguration" needs a Javadoc entry.
Also the configuration file name should be referenced symbolically.
5. The Javadoc entry for method "outputConfiguration" needs updating with @param spec.
Also option names should be symbolic.
6. Method "password" should use symbolic values for exit codes.
Also an error return should be made from the last two catch block in the method instead of dropping through and returning success.
7. In the method "type" type names should be symbolic as should option names.
Also is the character '/' in the constant "/.*" intended to be interpreted as a path component separator? If so it should be represented as the default platorm separator.
8. In the method "user" exit status should be symbolic.
Also the user permissions should be given symbolic values.
9. The method "within" should use symbolic names for options and the configuration file.
It should also use symbolic values for exit codes.
org.eclipse.stellation.workspace.Connection
1. Javadoc for the "open" method need updating. Method no longer returns a value.
Also should use symbolic names for options.
2. Method getHandle needs Javadoc @throws specs added.
3. Method "getAccessPoint" should use symbolic option names.
org.eclipse.stellation.workspace.Context
1. The "getComment" method needs updated Javadoc with @param, @return and @throws specs.
If you prompt for a comment in command line mode, you should tell the user how to end the comment. Many users have forgotten or do not know how to type an EOF from the keyboard. As a quick check I asked our Unix developers where I work and not one could remember how to do this. I couldn't either!
org.eclipse.stellation.workspace.Create
1. "create" method
Should this not be made a static method of workspace.Project? As it stands it simply looks like a kuldgy way of getting a non-class method.
Given that all the necessary functionality is in Project is there really any need for a "do nothing" class just to support the create command. If you were going to add the ability to create more types of objects than simply projects than this class would make sense.
org.eclipse.stellation.workspace.Differ
1. The "command" method should use symbolic names for options.
2. The Javadoc comment for the "compare" method is a little misleading. It should make clear that what is being compared is the text artifacts in the current workspace with a branch of the same project in the repository. Part of the confusion rises from the fact that a project object represents a branch and it's materialization in a workspace, not an entire project. Also it is missing a @param spec for the second project.
The names for the commands to compare should be made symbolic.
3. The Javadoc for the method "differ" needs to be updated to reflect the new calling sequence.
Incidentally I find it confusing to use a method that differs only in case from the constructor. It is too easy for a maintenance programmer to get confused.
The method should use symbolic names for the command names.
4. A Javadoc entry is needed for "diffOnly".
It should also use symbolic option names.
5. The method "getRevisionProject" looks as if it belongs in the Project class. It offers no functionality that is specific to the differ command but simply implements some functionality that uses the Project class.
6. Method "getStateProject" nedds a Javadoc update - needs @throws spec.
Again this method really looks like it belongs in the Project class.
org.eclipse.stellation.workspace.Exec
No comments on this class.
org.eclipse.stellation.workspace.Export
1. Constructor need Javadoc update. Parameter name changed.
2. Method "command" should use symbolic values for option names.
3. Method "exportDirectory" should use symbolic names for options.
4. Method "exportZip" need Javadoc update for @param and @throws specs.
Also should use symbolic names for option values.
Should use platform default dpath component separator rather than hard coded constant.
org.eclipse.stellation.workspace.Files
1. Both constructors need Javadoc.
2. Javadoc comment for "closeContentDirectories" is misleading as perhaps is the method name. The content directores are being deleted, not closed.
3. The two file copy methods will benefit significantly from an increase in the buffer size used from 4096 to 65536. The reason for this has to do with the caching strategies used with both Windows and Linux will get a strong hint to read ahead aggressively which is exactly what you want when doing file copies. The only cost is a larger tan necessary buffer with small files, but that cost is less than the overhaed involved in discovering the file size and tuning the buffer size.
4. In the method "emptyArchive" should it be an error to encounter a sub-directory in the archive directory? If not is it intentional to leave any discovered sub-directories alone.
5. The method "find" is not going to work properly on Windows systems unless Cygwin and the GNU utilities are installed. For the moment, this is acceptable, but over the long term it seems a bit much to have to install Cygwin simply to get access to "find" and "diff", and at some point alternates for these utilities should be found or developed.
6. The method "findFileInfo" can be given an alternative implementation using Java IO only in a Windows environment. The reason for this is that in Windows you are only interested in regular files and directories and do not have to worry about executable files or symbolic links. This would remove the need for the "find" utility in Windows since the only use Stellation seems to make of it is to classify the workspace files. Of course, if you are willing to allow JNI, then there is no need for the "find" utility in a Unix environment either.
This method should be given a standatd Javadoc entry to pick up the throws specification.
7. The method "getArchiveDirectory" should be given a standard Javadoc entry to document exception and return types.
8. The method "getCanonicalPath" will not work correctly as it stands since it will not properly normalize all Windows paths. The problem is that in Windows both '\' and '/' are allowed as path component separators in all places except the command line and can be intermixed in the same path. The easiest solution would be to add a first pass that converts all instances of path separators into the default path separator for the platform. This will also fix numerous downstream problems where other methods try to parse path strings to identify directories.
This routine also needs to deal with UNC prefixes and/or drive letters on Windows
9. The method "getCheckDirectory" needs a full Javadoc entry for return type and exception.
10. The method "getCheckProjectFile" needs a full Javadoc entry for return type and exception.
11. The method "getContentDirectory" can easily be modified to not require looping to determine the next directory name to allocate. Simply get a list of the files and directories in the context root directory and use the list size as the file name for the next directory. This will be more efficient than testing files one at a time.
12. The Javadoc for the method "getContentRootDirectory" should be updated with the return type.
13. Should the method "getCurrentDirectory" throw an exception if it is not defined. This represents a serious software error and I doubt that any of this methods consumers are prepared to handle a null pointer.
14. The method "getExecutables" should return an empty StringSet immediately on Windows since the Unix concept of executable file attribute is not applicable. I don't think that it is necessary to simulate this because even if a Windows executable gets stored on a Unix file system it is not likely to be executed!
The Javadoc entry also needs upgrading.
15. The method "getLinkTarget" should return a null immediately on a Windows platform since soft links are not available at application level.
16. The method "getMembers" should check against the project path symbolically rather than using hard coded path.
17. The method "getName" is not guaranteed to work on Windows unless the path is in the form of an absolute canonical path. This is because if the path has not been normalized you cannot guarantee what path separator is present. For safety's sake all paths should be normalized to use the default path component separator for the active platform.
The Javadoc entry needs upgrading for parameter specification.
18. The method "getParentName" is not guaranteed to work on Windows unless the path is in the form of an absolute canonical path. This is because if the path has not been normalized you cannot guarantee what path separator is present. For safety's sake all paths should be normalized to use the default path component separator for the active platform.
The Javadoc entry needs upgrading for parameter specification.
19. The method getTime can be refactored a bit to eliminate redundant code when the Java File class can be used.
20. The Javadoc needs upgrading for revised parameter list.
21. Fix the Javadoc for "getWorkspaceDirectoryPath". It returns a path, not a directory.
22. Upgrade Javadoc for "getWorkspaceDirectoryFiles" for parameter list.
23. Upgrade Javadoc for "guardFile" for parameter list.
24. Upgrade Javadoc for "getRelativePath(String) for altered parameter name.
25. The ASCII tests in "getTypeFromContents" are only valid for basic ASCII chaacter sets. Ultimately, Stellation should be able to deal with files that use any encoding and any character set, but this is a substantial area that is better dealt with later.
26. The Javadoc comment for "getXMLFile" is misleading. The method simply builds a File object for the XML file. It does not read it. Javadoc also needs @param spec.
27. In the method "getWorkspaceFiles" the direcctory name for the project directory should not be hard coded.
28. The Javadoc for "inputXMLFile" needs a @param spec.
29. The Javadoc spec for "isLink" needs upgrading.
30. The Javadoc for "isExecutable" needs upgrading.
31. The Javadoc for "isPreserved(File)" needs to be revised to reflect the return value and have a @param spec added.
32. The Javadoc for "isPreserved(String)" needs to be revised to reflect the return value, new exception behaviour and have a @param spec added.
There should be symbolic definitions for the types.
The Perl compiler should be configured to generate case insensitive patterns on a Windows platform.
33. The Javadoc for "isWithinWorkspaceDirectory" needs an @param spec.
34. The Javadoc for isWorkspaceFile needs upgrading.
35. "isWorkspacePath" will only work reliably under Windows if path component separators have been normalized to the platform default. Javadoc needs upgrading to add @param spec.
36. The method "moveFile" needs Javadoc.
If the call "renameTo" fails should revert to doing a file copy/delete sequence. There are too many legitimate cases on both Windows and Unix to give up without a final try.
37. The method "normalizeText" neds the Javadoc entry revised to reflect new implementation.
I have serious doubts about the usefulness of this operation. It is costly and given that all internal operations are done on text artifacts where line termination characters have been removed during the construction process I don't see the benefit.
38. There is no Javadoc entry for the method "outputProjectDocument".
39. The method "removeFile" returns one of a set of small integers as a return code. This seems to be a fairly common idiom in Stellation. Should these values not be given symbolic names and used consistently throughout the code base?
40. The Javadoc for method "updateFileInfo" needs upgrading with @throws.
org.eclipse.stellation.workspace.Fork
1. The method "fork" needs a Javadoc update. Calling sequence has changed.
Also LogManager constants should be symbolic.
org.eclipse.stellation.workspace.GetProperty
1. The constructor Javadoc needs updatinjg for parameter name change.
2. The method "command" should use symbolic values for exit codes and options.
3. The Javadoc for method "getAllProperties" needs updating for changed calling sequence.
Also symbolic values should be used for exit codes.
4. The method "getProperty" should use symbolic values for exit codes.
Also the comments indicate an uncertainty over the return code set when a named property is not present. This should be resolved but given that this method is called from the command line it is probably better to leave things as they are so external scripts can know what is happening.
org.eclipse.stellation.workspace.List
1. The method "command" should use symbolic values for options, command qualifiers and exit codes.
also should not the return code from lower level calls be returned since this may be the only way for a script to know that a problem exists.
2. The method "listBranches" needs a Javadoc update for changed parameters and exceptions. It should also use symbolic values for exit codes.
3. The method "listComment" needs a Javadoc update for exceptions. It should also use symbolic values for options and command modifiers as well as for exit codes.
4. The method "listFiles" needs a Javadoc update for for parameters. It should also use symbolic values for options and exit codes.
5. The method "getProjectName" should use symbolic names for command modifiers.
6. The method listStatus should use symbolic names for options keys and exit codes.
7. The method "listOptions" need a Javadoc update for return code and exceptions. It should also use symbolic values for exit code.
8. The method "listProjects" should use symbolic names for exit codes.
9. The method "result" needs Javadoc update for parameters and exception. Also keys should be referenced symbolically.
10. The method "selectError" should use symbolic names for exit codes.
11. The method "listVersions" needs Javadoc update for parameters and exceptions.
Also should use symbolic names for exit codes and default branch name.
org.eclipse.stellation.workspace.Location
1. The constructor Javadoc should be updated with @param spec. Also, database names should be referenced symbolically and other location option names should also be symbolic. For the longer term this method probably needs to be initialized with static tables defining protocol specific options that may be present in the location string. This can be done during database protocol loading.
2. The Javadoc for method "getHost" needs updating since this call is valid for some local protocols such as MySQL. However, it is probably better to replace the parameter specific calls "getHost" or "getPort" with generic calls that take the name of the option as a parameter and always return a string. This mechanism will fit with the table driven parameter identification mechanism.
3. A specific variable to identify a local protocol is now required since some local protocols may use socket connections and need host and port parameters.
org.eclipse.stellation.workspace.LogEntry
1. The constructor "LogEntry(int)" should use symbolic names for attribute names.
2. The method "add" should use symbolic names for attribute names.
3. The method "getAction" should use symbolic names for actions.
4. The method "getDate" should use symbolic names for attributes
5. The method "getID" should use symbolic attribute names.
6. The method "getName" should use symbolic attribute names.
7. The method "getSignature" should use symbolic attribute names.
8. The method "getState" should use symbolic attribute names.
9. The method "getTime" should use symbolic attribute names.
10. The method "setState" should use symbolic attribute names.
11. The method "setTime" should use symbolic attribute names.
org.eclipse.stellation.workspace.LogManager
1. The constructor needs a Javadoc entry. Also node names should be symbolic.
2. The method "addEntry" should use symbolic names for action codes.
3. The method "getLogEntry" needs a Javadoc update for @param spec.
It should also use symbolic names for argument kind and commands.
4. The method "listLog(String)" need a valid Javadoc entry for @param
5. The method "ListLog(StringList)" need a Javadoc @param spec. It should also use symbolic exit code names. It should also use symbolic names for commands and command abbreviations.
6. The method "write" should use symbolic names for the XML elements it references.
org.eclipse.stellation.workspace.Merge
1. The Javadoc for the constructor should be updated. A parameter name has changed.
2. The Javadoc for the "command" method should be updated to add an @return spec. It should also use symbolic names for log actions and command names.
3. The method "performMerge" needs a Javadoc update to add a @param spec. It should use symbolic names for standard project directory names.
4. The method "cleanCompound" should use the platform default path component separator in the logging statements for tidiness and consistency when normalized path names are used.
org.eclipse.stellation.workspace.Project
1. The constructor "Project(Workspace, AgentSpecList, Document)" neds a Javadoc update to reconcile the name of the third parameter.
2. The constructor "Project(Workspace)" should use symbolic names for the input channel kind.
3. The constructor "Project(Workspace,Integer)" should use symbolic names for attribute keys and input channel types.
4. The constructor "Project(Workspace, Integer, IntegerSet)" should use symbolic names for the input channel set.
5. The constructor "Project(Workspace, Long, IntegerSet)" should use symbolic names for the input channel set.
6. The method "add(Artifact, StringMap)" should use symbolic names for attribute keys.
7. The method "add(StringMap)" should use symbolic names for attribute keys.
8. The method "add(String, StringMap)" should use symbolic names for attribute keys.
9. The method "addAttributes" should use symbolic names for attribute keys.
10. The method "buildArtifacts" should use symbolic names for attribute names and XML tags.
11. The method "buildAttributes" needs a Javadoc entry. It should also use symbolic names for attribute keys.
12. The method "buildDirectory" should use symbolic names for XML tags and attribute keys. It also needs an updated Javadoc entry with @param and @throws specs.
13. The method "buildParents" should use symbolic values for XML tag names. It also needs a Javadoc entry.
14. The method "buildPaths(String)" needs a Javadoc entry.
The variable _pathMap is problamatic as it stands. Since it is going to contain file paths, it needs a custom comparator to ensure that compares on the paths are case sensitive on Unix systems and case insensitive on Windows systems.
15. The method "buildPaths(CompoundArtifact, String)" needs a Javadoc entry.
16. The method "buildPropertyElements" should use symbolic values for XML tags.
17. The method "contains(Integer)" should have the Javadoc entry updated to add a @param spec.
18. The method "contains(String)" needs updated Javadoc to include a @param spec.
19. The method "getArtifactAttributes" need a Javadoc update to add @param and @return tags.
20. The method "getEntryList" need the Javadoc parameter spec reconciled with the argument name.
21. The method "getID" need the Javadoc entry updated with @param and @return specs.
22. The method "getModifiedArtifacts" should use symbolic names for attribute keys.
23. The method "getName" needs an updated Javadoc entry with @param and @return specs.
24. The method "getParent" needs a Javadoc entry.
25. The method "getPath" needs the Javadoc updated with a @param and @return spec.
26. The method "getProjectSignature" need the Javadoc updated with a @throws spec. It should also use symbolic names for attribute keys. There is a problem with the computation of the project signature. It uses the Java algorithm to hash a string. The problem with this is that hash production algorithms are only required to provide a reasonable expectation that a collision will not occur. In other places you use the MD5 hash which has much better behaviour in this respect. For strong one-way hash algorithms, the probability that two input streams will generate the same hash is roughly equivalent to 1 in 2^number of hash bits. So an MD5 hash will give you odds of 1 in 2^64. Probably the best hash algorithm currently available is the SHA-1 hash. It uses a 20 byte hash and so yields a 1 in 2^160 probability of generating a collision. The donside of course is that it requires 20 bytes of storage as opposed to 8 bytes for your current hashes.
If you are interested in using this algorithm, there is an excellent open source implementation in the Bouncy Castle cryptographic library.
This method should also use symbolic names for attribute keys.
27. The method "isDirectory" needs a Javadoc update to add a @param and a @return spec.
28. The method "mv" needs the Javadoc entry updated for changed calling sequence. It also needs a @throws spec.
29. The method "remove" needs the Javadoc updated. The name of one of the parameters has changed.
30. The method "removeEntry" need a Javadoc entry.
31. The method "setInput" should use symbolic names for input channel kind.
32. The method "setProjectBranch" has an editing errorcheckError" in the Javadoc.
33. The method "setProperty" needs the Javadoc updated for a changed calling sequence.
Also would it not be prudent to add a second to the "propertyModificationTime" to ensure that it is always different to the oroginal time.
34. In the method "update" the discussion on hashes in point 26 for this class is relevant. Note that the code for signature computation can be significantly simplified if you use SHA_1 which is guaranteed to produce a non-zero hash.
Also the discussion in the Files class point 37 on the need for text file normalization is relevant.
Finally symbolic names should be used for attribute keys.
35. in method "write(OutputStream)" there is a comment about PrintWriter error handling. Although PrintWriter does not throw exceptions it does provide a "checkError" method that allows errors to be detected and if so then an exception can be thrown.
org.eclipse.stellation.workspace.SetProperty
1. The constructor Javadoc needs updating for a changed calling sequence.
2. the method "command" should use symbolic names for options and for exit codes.
3. The method "setProperty" needs an updated Javadoc for a changed parameter list.
org.eclipse.stellation.workspace.Svc
1. The method "check" needs a Javadoc entry.
2. The method "command" should use symbolic names for commands and exit codes.
3. The method "debug" needs a Javadoc entry.
4. The method "defaultOptionValues" should use symbolic names for option values.
5. The method "findWorkspaceDirectory" needs a Javadoc entry. It should also use symbolic names for attribute keys, exit codes and for the workspace directory name.
Also there is a "TODO" comment on one statement in this method. Is this still relevant and if so what needs to be done?
6. The method "getCommandSelector" should use symbolic names for command names.
7. The method "help" needs its' Javadoc entry replaced. It mus have arrived by cut and paste.
8. The method "main" breaks conventional usage in the Javadoc entry. The method does not return a value, instead it calls a system function to set the exit code. This should be documented but in the description, not in a @returns tag.
9. The method "optionSelector" should use symbolic names for option names and option types.
10. The Javadoc description is misleading for method "printException". The method operates on an exception period. It knows nothing about throw or catch operations in other methods and in fact may be called before an exception is first thrown in some circumstances.
11. The method "readConfiguration" needs a Javadoc entry.
12. The method "run" needs a Javadoc update to add a @return spec. It should also use symbolic names for option types, attribute names and exit codes.
Also there seems to be a bit of fragility in the use of the "simple" option type. Currently the only option that has this type is "help". The semantics of the type are not specified but the code uses the type as a signal to issue the help message rather than directly recognizing the option. If the "simple" type has real meaning than it is possible that other options might be added in the future with this type and the existing code would then break;
org.eclipse.stellation.workspace.Test
1. The constructor Javadoc needs updating for a changed parameter name.
2. The method "command" should use symbolic names for commands and exit codes.
3. The method "selectError" needs a Javadoc entry. It should also use symbolic names for exit codes. Also what is the meaning of always returning 2 as the exit code? Why have a return code if the value is never different?
org.eclipse.stellation.workspace.Workspace
1. in the constructor "Workspace(Context, File)" the Javadoc needs updating for a change in parameter list.
2. In constructor "Workspace(Context, File, File)" symbolic names should be used for XML tags and for standard workspace file names.
3. In method "getAgents" symbolic names should be used for standard file names.
4. In method "getAttribute" the Javadoc needs updating with a @param spec. Also the description is incorrect. It returns the value for an attribute name.
5. In method "getHeadVersion" the Javadoc needs updating with an additional exception.
6. In method "getNumberOfStates" the Javadoc needs a @return spec added.
7. In method "getProjectDocumentFile" the XML file suffix should be symbolic.
8. In method "getRevisionBranch" the Javadoc needs updating with additional exception. Also the description need editing to remove stray '*';
It should alsouse symbolic names for options.
9. In the method "getRevisionVersion" the Javadoc needs updating with an additional exception.
It should also use symbolic names for options.
10. In the method "getState" the Javadoc should be updated for a changed calling sequence and a @return spec should be added.
11. In the method "getVersionList" the Javadoc should be updated to add another @throws spec.
12. In the method "makeConfigAgent" symbolic names should be used for the XML tags.
13. In the method "makeConfigDocument" symbolic names should be used for attributes and XML tags. Also an additional @throws spec if needed.
14. In the method "resetCounters" the Javadoc needs a @throws spec.
org.eclipse.stellation.workspace.WorkspaceException
1. Both constructors need Javadoc entries.
