Is the switch in the listing inside the loop a good style

Question

I have a problem with implementing something in Java, and I think Enums are part of it (or maybe just me who made them part of the problem.

So, I have a model class, let's say it has 3 fields:

class ConferenceInfo { String name; Date date; String location; public boolean compareByName(ConferenceInfo other) { return getName().equals(other.getName()); } public boolean compareByDate(ConferenceInfo other) { return getDate().compareTo(other.getDate()) == 0; } public boolean compareByLocation(ConferenceInfo other) { return getLocation().equals(other.getLocation()); } } 

I have a list of pairs of these objects that I need to compare:

 List<Pair<ConferenceInfo, ConferenceInfo>> 

Now it would be quite easy if I could just override the equals method, but in this case it is a little different - I need to compare these objects by the values ​​of their fields and display the number of them that differ. For this, I created an enumeration:

 public enum FieldType { NAME, DATE, LOCATION } 

and created a method (in my Measurement class that has the above list of pairs) that takes FieldType as a parameter and does something like this:

 for (Pair<ConferenceInfo, ConferenceInfo> result : results) { ConferenceInfo first = result.getFirst(); ConferenceInfo second = result.getSecond(); switch (field) { case NAME: if (!first.compareByName(second)) { differentValues++; } break; case DATE: if (!first.compareByDate(second)) { differentValues++; } break; case LOCATION: if (!first.compareByLocation(second)) { differentValues++; } break; } } 

This works, but I wonder if the way I encoded is a good practice, or should I write it differently .. I don't seem to like the switching inside the loop, but maybe it's just me :)

Answer 1

Instead of your switch, you can use polymorphism:

 public enum FieldType { NAME { @Override public boolean areEqual(ConferenceInfo c1, ConferenceInfo c2) { return c1.compareByName(c2); } }, DATE { @Override public boolean areEqual(ConferenceInfo c1, ConferenceInfo c2) { return c1.compareByDate(c2); } }, LOCATION { @Override public boolean areEqual(ConferenceInfo c1, ConferenceInfo c2) { return c1.compareByLocation(c2); } }; public abstract boolean areEqual(ConferenceInfo c1, ConferenceInfo c2); } 

And then in your code:

 for (Pair<ConferenceInfo, ConferenceInfo> result : results) { ConferenceInfo first = result.getFirst(); ConferenceInfo second = result.getSecond(); if (!field.areEqual(first, second)) { differentValues++; } } 

This has the added benefit that if a new enum value appears, you will not have the risk of forgetting to add a case to the switch.

Answer 2

Not sure if I missed something, but in this case I would execute the method in the simplemodel class and check for different values:

 class SimpleModel { ... int differentFields(SimpleModel obj){ int differentValues = 0; if (!this.compareByName(obj)) { differentValues++; } if (!first.compareByDate(second)) { differentValues++; } if (!first.compareByLocation(second)) { differentValues++; } return differentValues; } } 

and in a method that compares pairs, use it

 for (Pair<ConferenceInfo, ConferenceInfo> result : results) { ConferenceInfo first = result.getFirst(); ConferenceInfo second = result.getSecond(); first.differentFields(second); }