RE: [stellation-res] Code Audit for Workspace package

["Jonathan Gossage" <jgossage@xxxxxxxx>]

> >-----Original Message-----
> >From: stellation-res-admin@xxxxxxxxxxxxxxx
> >[mailto:stellation-res-admin@xxxxxxxxxxxxxxx]On Behalf Of Mark C.
> >Chu-Carroll
> >Sent: September 2, 2002 4:01 PM
> >To: stellation-res@xxxxxxxxxxxxxxx
> >Subject: Re: [stellation-res] Code Audit for Workspace package
> >
> >
> >
> >I don't want you to feel ignored... The reason that there's been no
> >acknowledgement of this audit yet is because Dave Shields, who's the
> >guy responsible for the workspace code, is away, taking his daughter
> >to college.
> >
> >One quick comment: I noticed a couple of places where you ask "Can
> >you be sure that this code will only be used from the command line". The
> >answer to that is "yes". The behavior of a system that's well integrated
> >into the Eclipse IDE is sufficiently different from that of a
> >well-designed command line tool that we're going to keep them separate.
> >(There's some significant overlap right now, but we're going to be
> >gradually eliminating that.)
> >
> >	-Mark

No problem with feeling ignored. I thought that something like this would be
necessary. With regard to workspace management in the Eclipse and command
line environments, I believe it is worth some thought to try and see if
there is not a core set of functionality that can sit on top of either the
Eclipse workspace objects or standard command line objects. If you can do
that, perhaps using the same interface definition as Eclipse uses, you could
preserve a substantial portion of the workspace code base. It seems a bit
rude to have to duplicate functionality to support the two environments.
After all they are both file based workspace environments so the conceptual
pardigms are not totally different even if the details are.

Jonathan

> >
> >On Mon, 2002-09-02 at 15:43, Jonathan Gossage wrote:
> >> 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.
> >--
> >Mark Craig Chu-Carroll,  IBM T.J. Watson Research Center
> >*** The Stellation project: Advanced SCM for Collaboration
> >***		http://www.eclipse.org/stellation
> >*** Work Email: mcc@xxxxxxxxxxxxxx  ------- Personal Email:
> >markcc@xxxxxxxxxxx
> >
> >
> >_______________________________________________
> >stellation-res mailing list
> >stellation-res@xxxxxxxxxxxxxxx
> >http://dev.eclipse.org/mailman/listinfo/stellation-res