• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Trouble with warning for identical expression while checking major / minor version for io

 
John Vorwald
Ranch Hand
Posts: 139
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
For class IO, to keep track of the ability to read previous versions of the class, I use the following to determine if the file can be read



My problem is with the warning message comparing "minor_version == 5.0". It gives a warning "comparing identical expression". I understand the warning, but can I change the code to remove the warning?
 
Steve Luke
Bartender
Posts: 4181
21
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
John Vorwald wrote:

My problem is with the warning message comparing "minor_version == 5.0". It gives a warning "comparing identical expression". I understand the warning, but can I change the code to remove the warning?

Yes, you can remove the minor_version == 5.0 check because you know for a fact that it will always be true.
 
Steve Luke
Bartender
Posts: 4181
21
IntelliJ IDE Java Python
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That seems like a lot of work and a pretty poor design. First, I don't think your versions should be doubles. They should either be ints or strings. Comparing doubles is a pain in the buttocks, and you don't gain anything by using them.

Second, you have a check---() method which doesn't return a boolean value, but instead throws an exception when the check is bad. This seems plain wrong to me (maybe it makes sense in context, but not as shown here).

Third, you start with doing what seems reasonable:
'Is the major version my major version?'
'Is the minor version my minor version?'
But then you degrade into a bunch of magic numbers and redundant comparisons. For example, your major_version is 1.0. Your first comparison is major == major_version. But if that fails, your else if is to check major == 1.0 which is precisely the same comparison. You make that magic-number comparison inside the else if when minor == minor_version fails as well, and it is redundant there as well, since the major == major_version was already true.

I think you have to go back and think about what cases are supposed to work and what cases aren't. Start from scratch. Here are a couple of examples (remember I don't know any of your requirements, so these are naive - but show that simpler is better, and named constants are more expressive).



Of course, I would just keep an incrementing counter for versions and let the major/minor stuff be left for display purposes.

 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic