Design directions:

  1. Move from using Result to CommandResult throughout the system.

  2. Whenever updating a command, extract command implementation into a separate class. Look for grouping in the parameter list and consider making them into one class (see Credential object).

  3. Commands should never reference gfsh. Whenever necessary, use CommandContext instead of referencing Gfsh directly.

  4. Get rid of Gfsh’s own flavor of logging completely. Use LogService.

  5. Consider breaking CommandContext into CommandOutputContext and CommandInputContext.

  6. Rule: each command class should only have one command

  7. Rule: command should throw exception rather than return system error result. It should only return the InfoResult or UserErrorResult (any error that doesn’t have a stack trace. Like user specify a dir that doesn’t exist, a member name that doesn’t exist…).

  8. CommandProcessor and GfshExecutionStrategy should use CommandExecutor execute the commands.

  9. CommandExecutor should be the one place that would catch general exceptions thrown by each command.

  10. GfshExecutionStrategy catches the AuthorizationException and other exceptions happening in the transport, anything that escaped CommandExecutor.

  11. Use a single HttpOperationInvoker and a single Controller class to handle all command over http requests.

  12. Rule: whenever we touch a class, add a true unit test for it. Attempt to instantiate an instance by itself.

  13. Investigate why the CommandResult before and after the transport are different.

  14. For http command response, any commands that would result into a file download, return an FileInputStream, otherwise return a StringInputStream. On the client side, it looks into the response content-type header to decide whether to write the stream into a file or turn it into a Result object.

  15. A command can only be either online or offline, can’t be both. (currently we use annotation to denote it, consider using different base class). Only offline commands should have access to the CommandContext (gfsh).

  16. Move toward always using Jackson json instead of home-brewed json or org.json.

  17. Reduce nesting by returning early. Replace conditional guard clause.

  18. Don’t use java Asserts in your code. Use Design by Contract, documenting the pre-condition would be enough.

  19. Try to minimize the usage of Null.


To Fix:

  1. Move the tests using TestableGfsh to use HeadlessGfsh

  2. remove try catch block in each commands

  3. consolidate CommandResult and CommandResponse

  4. Annotation for test constructor

  5. gfsh uses LogWrapper while server uses LogService, refactor gfsh to use logService.

  6. ConnectCommand: re-write the do-while loop and lump properties using an SslConfig object for later use.

  7. CliMultiStep

  8. In connect command, use ping instead of index request to check authentication.

  9. In GfshExecutionStrategy, fix the way interceptors are called.

    1. Implement a no-op interceptor

    2. Make the declaration a class instead of a string

    3. Execute on remote should just be :

      1. Create an interceptor

      2. Pre-execute, execute, post-execute.

  10. Have more tests to document the structure of the json of CommandResult.

  11. Clean up CommandResult:

    1. Reduce duplication inside the object construct.

    2. Consider separating result content from result display.

    3. Lines contains newline?

    4. Consider returning iterators for the lines.

  12. Further reduce the argument list for ConnectCommand by grouping all ssl configuration together.

Lesson learned:

  1. completely single-flavor test. A test should only test a limited set of behavior.

  2. Use compiler to help you refactor, compiling errors means points of dependency.

  3. TDD makes design better because it help you understand how your code will be used.

  4. test problems indicates design problems.

  5. Narrow down your scope when refactor and pay attention to what it is v.s what we want.

  6. Define behaviors using tests. Tests before coding.

  7. Do not put too much into one object. Consider separation of responsibility instead of convenience. (See CommandResult)

  8. Change one thing at a time and run tests frequently, and commit separately.

  9. Simplifying code can involve talking to user community to get feedback. Look for opportunities to simplify behavior and implementation.

  10. Mobbing when hashing out design details, not just whiteboarding, but writing code and tests together, and setting design directions together.

  11. Having turns driving when mobbing.

  12. Break often when mobbing, 10 min hourly.