Thanks Yongjun Zhang for the insightful suggestion. This motivated my v2 patch.
The pain in the code to handle different option combinations comes from the fact that, for each option we may validate and set it individually. This is not a clear way as 1) not efficient, 2) not well defined, and 3) error prone.
- For point 1) we validate the options multiple times which is not needed or scalable.
- For 2) some of the options are set after validation while the other options are set without validation. Distributing the decision to validate or not to validate across all the setters smells bad to me.
- For 3), when we validate an option, chances are that its dependent option B is not set yet. This implies that the order of setting options have to be carefully chosen, leading to fragile code snippet. Take syncFolder and skipCRC for example, skip CRC is valid only with update options, and if we set (and thus validate) skipCRC before setting syncFolder option, the validation will fail, even if both of them are provided in the command line.
I think a better way is to validate all the options only once after all the options are set, i.e. a central validation method. Moreover, the parser is to parse the options and should not handle the validation of option combinations explicitly, if it's possible to delegate the work to validate() method of DistCpOptions. Of course, if there is any parsing errors of a single option (eg. only one snapshot is provided for the -diff option), the parser should throw the IllegalArgumentException directly.
What's your thought?
Ping Jing Zhao for more input.