1

JSON :

"ABCD": [  
      {  
        "xyz": 3,  
        "abc": 4,  
        "date": {  
          "start": 1462341600000,  
          "stop": 1462513680000  
        }
      }
      ...similar sub parts
    ]

Datatype defined : I have tried to define a corresponding datatype for the json as follows [Ignoring other field as of now] :

public class Test {
    TestDate testDate = new TestDate();
    private Long start = testDate.startTime;
    private Long stop  = testDate.expiryTime;

    public class TestDate {
        private Long startTime;
        private Long expiryTime;
        // getter and setter implemented here 
    }
}

Now, what I am confused with is how to override the equals method to suffice the check for start and stop parameters of the JSON.

Case 1 : Comparing the variables start and stop declared inside the Test class and assigned with TestDate corresponding parameters. As in :

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Test test = (Test) o;
        // Below is what I am concerned about
        if (stop != null ? !stop.equals(test.stop) : test.stop != null) return false;
        if (start != null ? !start.equals(test.start) : test.start != null) return false;
    }

OR

Case 2 : Simply use the object testDate created within the Test class for comparison. As in :

if (testDate != null ? !testDate.equals(test.testDate) : test.testDate != null) return false;

Doubts :

  • Does the Case 2 even suffice the Case 1?
  • If not, whats the difference?
  • If yes, which one of these is a better practice?

Looking for more stats to why use Case 2 over Case 1 in terms on code complexities and re-usability.

Naman
  • 27,789
  • 26
  • 218
  • 353
  • Case 2 does suffice case 1 - but only if the same logic re start/stop is moved to a TestDate.equals() method. I am assuming that is implicit in your Case 2. – haggisandchips May 25 '16 at 07:38
  • Why do you want to override equals? Note that if you do so you will also have to override hashCode... Generally I try to avoid overriding these methods because it's hard to get right and makes code harder to maintain. – Adriaan Koster May 30 '16 at 09:36
  • @AdriaanKoster : I have overriden `hashCode` as well. And there is no constructor, `TestDate` is a class itself – Naman May 30 '16 at 10:16
  • 1
    Oops! Monday morning I guess :-D – Adriaan Koster May 30 '16 at 10:20

1 Answers1

1

The correct approach would be to let Test concern itself with its own fields, so it should be checking itself against o as you have done but in my opinion the start/stop fields shouldn't exist. After the o checks, your Test equals method should end with identity/null checks for testDate and if both are non null and not the same object then it should delegate to (and return) an equals implementation on the TestDate class.

So basically Case 2 is the correct approach but I'd rethink the existence of start and stop on Test. Also that Case 2 statement could easily be refactored to a simple return without the if - not sure I've ever seen a ternary ? as an if condition before ;). Horses for courses but I think the following is easier to read:

return (testDate == test.testDate) || (testDate != null && testDate.equals(test.testDate));

Having the start/stop fields on the Test class means that the implementation of TestDate is leaking into the Test class. if you decided to make some significant change to TestDate (perhaps storing the values as localised dates instead for the sake of argument) then you break the Test class under Case 1 but Case 2 would survive this so Case 2 is more robust.

Another consideration as well is the hashCode method - the problems above under Case 1 with the start/stop fields on Test are effectively doubled because you should really be implementing hashCode in a consistent manner.

As systems get more complex the time involved fixing issues such as this quickly becomes non-trivial and more prone to error.

haggisandchips
  • 523
  • 2
  • 11
  • Though I get the context of your answer. Still looking for more stats to support it. As in why use Case 2 over Case 1 in terms on code complexities and re-usability. – Naman May 30 '16 at 08:22
  • Not sure what you are after in terms of stats - ultimately this is a subjective view. The reason that I believe Case 2 is better is to do with encapsulation and separation of concerns. Having the start/stop fields on the Test class means that the implementation of TestDate is leaking into the Test class. if you decided to make some significant change to TestDate (perhaps storing the values as localised dates instead for the sake of argument) then you break the Test class under Case 1 but Case 2 would survive this so Case 2 is more robust. – haggisandchips May 30 '16 at 09:15
  • ... As systems get more complex the time involved fixing issues such as this quickly becomes non-trivial and more prone to error. – haggisandchips May 30 '16 at 09:17
  • Comments above added as part of answer. – haggisandchips May 30 '16 at 09:27