$begingroup$ I entirely disagree with the argument re: equals. The class name is Length. In all ways 1000 mm is equal to 1m and is equal to 3.28084 ft.
A length is simply a distance, and the distances are equal. The coin argument is equally disingenuous. A dollar is a dollar no matter how you add it up. BUT, this only applies to the concept of MONEY. If you want a coin collector's app, then you're not talking about Money at all. You're talking about discrete objects, with a value different than the face value.
$endgroup$–Dec 2 '15 at 16:30. $begingroup$ No, that's not correct at all. The CLIENT defines equality. If the client is an engineer, or a carpenter, then the only thing they care about is that 1 m = 1000 mm. In fact, I doubt there is anyone in the world who would want a program/class/framework that states that 1 m!=1000mm. It is factually incorrect.
Just because the coding decision checks off something on your smell list, doesn't mean it's bad. Limburger stinks, and delivers exactly as expected. A smell is a warning, not an error. $endgroup$–Dec 2 '15 at 17:45. $begingroup$ But the overriding principle is the Principle of Least Surprise/Astonishment. I would be astonished if 1 m!= 1000mm. And as a user of the class called length, the least surprising thing would be that two lengths that are equal, would be programmatically equal.
Don't be beholden to guidelines, they are not rules. Certainly the EASIEST way is not to override it. But easiest isn't always best. But yes, you should always ask the question 'Do I need to?'
But the utility of equals over some other non-overridden method is something I want. $endgroup$–Dec 2 '15 at 19:16.
Split ConversionsMain to muliple ClassesRight now, your ConversionsMain class gathers user input, reads conversion rules from a file, validates these rules, and of course starts your program. I would create separate classes for all these functionalities.Right now, it's not all that bad, but if your program gets extended, a class like this can grow quite big very fast.Conversion classYour Conversion isn't really a conversion (of one unit to another). I would probably call it Unit instead.And then I would create a Conversion class which holds two Units (currently this is a Set in your code). This class could also handle the actual conversion from one unit to another.Encapsulationunits in ConversionPath should better be private and accessed by a getter.getConversionPath(String unit1, String unit2, Map visited) should also be private, and probably the other getConversionPath method as well.Getting User InputYou are generally handling exceptions very well, but when gathering user input, this can happen: java.util.InputMismatchException. It would be best to catch this and inform the user of wrong input.It would also be helpful if the prompt gave an example of correct input.You could also add a hasUnit method to your ConversionMap class, that way, you could give feedback about a non-existing unit as soon as a user enters it.Reading Rules FileRight now, if a rule doesn't match the pattern, you throw an exception and the program quits.
I would prefer it if malformed lines were skipped, so that your program still works even if one line is wrong.Instead of throwing an exception, you could log the error in a file.Naming. as mentioned above, Conversion is not really a conversion and should probably be renamed. by convention, static final variable names should be all uppercase (so numberRegex should be NUMBERREGEX. Exceptions should have Exception in their name.