ptuomola commented on pull request #1006: URL: https://github.com/apache/fineract/pull/1006#issuecomment-641687223
@thesmallstar well... that's definitely true, but only IF there is a default case that we know will make sense for new cases that may be added AND / OR if we don't have a mechanism to alert the developer if they have forgotten to add a case for a new type they've introduced. But in this case, thanks to ErrorProne, we can do better than with just standard Java: we have a choice of not needing to worry about whether the default case will work for any new cases, because ErrorProne will warn us if we add a new case that isn't handled So my view would be: - If you can write a default case that is really "default" - i.e. the common way of handling all cases where no specific handling is needed - with all specific cases being "exceptions" from the default, then by all means let's have a default case - If there is no clear default - i.e. the switch chooses from different types of behaviour, with no clear way of saying what is the common way of doing things - then in my view the cases should be explicitly named. In this way, when some adds a new value for the enum, they are also forced by Error Prone to think about the handling for that case. In this PR, there are both types of cases: - We can argue that the default for account number prefix is that the account has no prefix, with specific cases then saying that specific types of accounts can have prefixes. So in this case a default of returning an empty list of prefixes makes sense. - But it doesn't make sense to that e.g. the default interest term is one month. Each interest term is different (one week, one month, one year) and there is no "default" we can assume will lead to a right behaviour. If we say that we use e.g. one month as default for everything we don't recognise, we will get wrong results for every new term that may be added. So in such case it makes sense to force the developer who adds a term to also add a new case in the routines that need to work on that term. Just my thoughts - happy to discuss... ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
