Skip to content

Commit d61dcf8

Browse files
author
richard.barnett
committed
Replace poor/naive impls of equals() & hashCode() with simpler & better
ones. In the worst case (Photo): - they write to System.out - they call the get*Image() & get*AsInputStream() methods which make http requests to flickr.com - .equals() is not reflexive since it tests .equals() on two distinct BufferedImage/InputStream objects - though they're built from the same url they're not equals().
1 parent 946c8e6 commit d61dcf8

8 files changed

Lines changed: 64 additions & 485 deletions

File tree

‎Flickr4Java/src/com/flickr4java/flickr/people/User.java‎

Lines changed: 7 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,15 @@
33
*/
44
package com.flickr4java.flickr.people;
55

6-
import com.flickr4java.flickr.contacts.Contact;
76
import com.flickr4java.flickr.contacts.OnlineStatus;
87
import com.flickr4java.flickr.util.BuddyIconable;
9-
import com.flickr4java.flickr.util.StringUtilities;
108
import com.flickr4java.flickr.util.UrlUtilities;
119

1210
import java.io.Serializable;
13-
import java.lang.reflect.InvocationTargetException;
14-
import java.lang.reflect.Method;
1511
import java.text.DateFormat;
1612
import java.text.ParseException;
1713
import java.text.SimpleDateFormat;
1814
import java.util.Date;
19-
import java.util.regex.Matcher;
2015

2116
/**
2217
* @author Anthony Eden
@@ -319,93 +314,18 @@ public boolean equals(Object obj) {
319314
if ((obj == null) || (obj.getClass() != this.getClass())) {
320315
return false;
321316
}
322-
// object must be User at this point
323-
User test = (User) obj;
324-
Class cl = this.getClass();
325-
Method[] method = cl.getMethods();
326-
for (int i = 0; i < method.length; i++) {
327-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
328-
if (m.find() && !method[i].getName().equals("getClass")) {
329-
try {
330-
Object res = method[i].invoke(this, null);
331-
Object resTest = method[i].invoke(test, null);
332-
String retType = method[i].getReturnType().toString();
333-
if (retType.indexOf("class") == 0) {
334-
if (res != null && resTest != null) {
335-
if (!res.equals(resTest))
336-
return false;
337-
} else {
338-
if (res == null && resTest == null) {
339-
// nop
340-
} else if (res == null || resTest == null) {
341-
// one ist set and one is null
342-
return false;
343-
}
344-
}
345-
} else if (retType.equals("int")) {
346-
if (!((Integer) res).equals(((Integer) resTest)))
347-
return false;
348-
} else if (retType.equals("boolean")) {
349-
if (!((Boolean) res).equals(((Boolean) resTest)))
350-
return false;
351-
} else if (retType.equals("long")) {
352-
if (!((Long) res).equals(((Long) resTest)))
353-
return false;
354-
} else {
355-
System.out.println("User#equals() missing type " + method[i].getName() + "|" + method[i].getReturnType().toString());
356-
}
357-
} catch (IllegalAccessException ex) {
358-
System.out.println("equals " + method[i].getName() + " " + ex);
359-
} catch (InvocationTargetException ex) {
360-
// System.out.println("equals " + method[i].getName() + " " + ex);
361-
} catch (Exception ex) {
362-
System.out.println("equals " + method[i].getName() + " " + ex);
363-
}
364-
}
317+
if (obj == this) {
318+
return true;
365319
}
366-
return true;
320+
User test = (User) obj;
321+
return id == null ? test.id == null : id.equals(test.id);
367322
}
368323

369-
/**
370-
* @see java.lang.Object#hashCode()
371-
*/
372324
@Override
373325
public int hashCode() {
374-
int hash = 1;
375-
Class cl = this.getClass();
376-
Method[] method = cl.getMethods();
377-
for (int i = 0; i < method.length; i++) {
378-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
379-
if (m.find() && !method[i].getName().equals("getClass")) {
380-
Object res = null;
381-
try {
382-
res = method[i].invoke(this, null);
383-
} catch (IllegalAccessException ex) {
384-
System.out.println("hashCode " + method[i].getName() + " " + ex);
385-
} catch (InvocationTargetException ex) {
386-
// System.out.println("hashCode " + method[i].getName() + " " + ex);
387-
}
388-
if (res != null) {
389-
if (res instanceof Boolean) {
390-
Boolean bool = (Boolean) res;
391-
hash += bool.hashCode();
392-
} else if (res instanceof Integer) {
393-
Integer inte = (Integer) res;
394-
hash += inte.hashCode();
395-
} else if (res instanceof String) {
396-
String str = (String) res;
397-
hash += str.hashCode();
398-
} else if (res instanceof Long) {
399-
Long lon = (Long) res;
400-
hash += lon.hashCode();
401-
} else if (res instanceof OnlineStatus) {
402-
OnlineStatus os = (OnlineStatus) res;
403-
hash += os.hashCode();
404-
} else {
405-
System.out.println("User hashCode unrecognised object: " + res.getClass().getName());
406-
}
407-
}
408-
}
326+
int hash = 83;
327+
if (id != null) {
328+
hash ^= id.hashCode();
409329
}
410330
return hash;
411331
}

‎Flickr4Java/src/com/flickr4java/flickr/photos/Editability.java‎

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,6 @@
33
*/
44
package com.flickr4java.flickr.photos;
55

6-
import com.flickr4java.flickr.util.StringUtilities;
7-
8-
import java.lang.reflect.InvocationTargetException;
9-
import java.lang.reflect.Method;
10-
import java.util.regex.Matcher;
11-
126
/**
137
* @author Anthony Eden
148
*/
@@ -43,43 +37,11 @@ public boolean equals(Object obj) {
4337
if ((obj == null) || (obj.getClass() != this.getClass())) {
4438
return false;
4539
}
46-
// object must be Editability at this point
47-
Editability test = (Editability) obj;
48-
Class cl = this.getClass();
49-
Method[] method = cl.getMethods();
50-
for (int i = 0; i < method.length; i++) {
51-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
52-
if (m.find() && !method[i].getName().equals("getClass")) {
53-
try {
54-
Object res = method[i].invoke(this, null);
55-
Object resTest = method[i].invoke(test, null);
56-
String retType = method[i].getReturnType().toString();
57-
if (retType.indexOf("class") == 0) {
58-
if (res != null && resTest != null) {
59-
if (!res.equals(resTest))
60-
return false;
61-
} else {
62-
// return false;
63-
}
64-
} else if (retType.equals("int")) {
65-
if (!((Integer) res).equals(((Integer) resTest)))
66-
return false;
67-
} else if (retType.equals("boolean")) {
68-
if (!((Boolean) res).equals(((Boolean) resTest)))
69-
return false;
70-
} else {
71-
System.out.println(method[i].getName() + "|" + method[i].getReturnType().toString());
72-
}
73-
} catch (IllegalAccessException ex) {
74-
System.out.println("equals " + method[i].getName() + " " + ex);
75-
} catch (InvocationTargetException ex) {
76-
// System.out.println("equals " + method[i].getName() + " " + ex);
77-
} catch (Exception ex) {
78-
System.out.println("equals " + method[i].getName() + " " + ex);
79-
}
80-
}
40+
if (obj == this) {
41+
return true;
8142
}
82-
return true;
43+
Editability test = (Editability) obj;
44+
return comment == test.comment && addmeta == test.addmeta;
8345
}
8446

8547
@Override

‎Flickr4Java/src/com/flickr4java/flickr/photos/GeoData.java‎

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
package com.flickr4java.flickr.photos;
22

3-
import com.flickr4java.flickr.util.StringUtilities;
4-
5-
import java.lang.reflect.InvocationTargetException;
6-
import java.lang.reflect.Method;
7-
import java.util.regex.Matcher;
8-
93
/**
104
* A geographic position.
115
*
@@ -76,43 +70,11 @@ public boolean equals(Object obj) {
7670
if ((obj == null) || (obj.getClass() != this.getClass())) {
7771
return false;
7872
}
79-
// object must be GeoData at this point
80-
GeoData test = (GeoData) obj;
81-
Class cl = this.getClass();
82-
Method[] method = cl.getMethods();
83-
for (int i = 0; i < method.length; i++) {
84-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
85-
if (m.find() && !method[i].getName().equals("getClass")) {
86-
try {
87-
Object res = method[i].invoke(this, null);
88-
Object resTest = method[i].invoke(test, null);
89-
String retType = method[i].getReturnType().toString();
90-
if (retType.indexOf("class") == 0) {
91-
if (res != null && resTest != null) {
92-
if (!res.equals(resTest))
93-
return false;
94-
} else {
95-
// return false;
96-
}
97-
} else if (retType.equals("int")) {
98-
if (!((Integer) res).equals(((Integer) resTest)))
99-
return false;
100-
} else if (retType.equals("float")) {
101-
if (!((Float) res).equals(((Float) resTest)))
102-
return false;
103-
} else {
104-
System.out.println(method[i].getName() + "|" + method[i].getReturnType().toString());
105-
}
106-
} catch (IllegalAccessException ex) {
107-
System.out.println("GeoData equals " + method[i].getName() + " " + ex);
108-
} catch (InvocationTargetException ex) {
109-
// System.out.println("equals " + method[i].getName() + " " + ex);
110-
} catch (Exception ex) {
111-
System.out.println("GeoData equals " + method[i].getName() + " " + ex);
112-
}
113-
}
73+
if (obj == this) {
74+
return true;
11475
}
115-
return true;
76+
GeoData test = (GeoData) obj;
77+
return longitude == test.longitude && latitude == test.latitude && accuracy == test.accuracy;
11678
}
11779

11880
@Override

‎Flickr4Java/src/com/flickr4java/flickr/photos/Note.java‎

Lines changed: 10 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,7 @@
33
*/
44
package com.flickr4java.flickr.photos;
55

6-
import com.flickr4java.flickr.util.StringUtilities;
7-
86
import java.awt.Rectangle;
9-
import java.lang.reflect.InvocationTargetException;
10-
import java.lang.reflect.Method;
11-
import java.util.regex.Matcher;
127

138
/**
149
* @author Anthony Eden
@@ -82,59 +77,24 @@ public boolean equals(Object obj) {
8277
if ((obj == null) || (obj.getClass() != this.getClass())) {
8378
return false;
8479
}
85-
// object must be Note at this point
86-
Note test = (Note) obj;
87-
Class cl = this.getClass();
88-
Method[] method = cl.getMethods();
89-
for (int i = 0; i < method.length; i++) {
90-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
91-
if (m.find() && !method[i].getName().equals("getClass")) {
92-
try {
93-
Object res = method[i].invoke(this, null);
94-
Object resTest = method[i].invoke(test, null);
95-
String retType = method[i].getReturnType().toString();
96-
if (retType.indexOf("class") == 0) {
97-
if (res != null && resTest != null) {
98-
// System.out.println("Note class: " + method[i].getName());
99-
if (!res.equals(resTest))
100-
return false;
101-
} else {
102-
// return false;
103-
}
104-
} else if (retType.equals("int")) {
105-
if (!((Integer) res).equals(((Integer) resTest)))
106-
return false;
107-
} else if (retType.equals("boolean")) {
108-
if (!((Boolean) res).equals(((Boolean) resTest)))
109-
return false;
110-
} else {
111-
System.out.println(method[i].getName() + "|" + method[i].getReturnType().toString());
112-
}
113-
} catch (IllegalAccessException ex) {
114-
System.out.println("equals " + method[i].getName() + " " + ex);
115-
} catch (InvocationTargetException ex) {
116-
// System.out.println("equals " + method[i].getName() + " " + ex);
117-
} catch (Exception ex) {
118-
System.out.println("equals " + method[i].getName() + " " + ex);
119-
}
120-
}
80+
if (obj == this) {
81+
return true;
12182
}
122-
return true;
83+
Note test = (Note) obj;
84+
// id doesn't change if you edit text (assume the same for move/resize), so test all attrs
85+
return areEqual(id, test.id) && areEqual(author, test.author) && areEqual(authorName, test.authorName) && areEqual(bounds, test.bounds)
86+
&& areEqual(text, test.text);
12387
}
12488

12589
@Override
12690
public int hashCode() {
12791
int hash = 1;
12892
if (id != null)
12993
hash += id.hashCode();
130-
if (author != null)
131-
hash += author.hashCode();
132-
if (authorName != null)
133-
hash += authorName.hashCode();
134-
if (bounds != null)
135-
hash += bounds.hashCode();
136-
if (text != null)
137-
hash += text.hashCode();
13894
return hash;
13995
}
96+
97+
private boolean areEqual(Object x, Object y) {
98+
return x == null ? y == null : x.equals(y);
99+
}
140100
}

‎Flickr4Java/src/com/flickr4java/flickr/photos/Permissions.java‎

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,6 @@
33
*/
44
package com.flickr4java.flickr.photos;
55

6-
import com.flickr4java.flickr.util.StringUtilities;
7-
8-
import java.lang.reflect.InvocationTargetException;
9-
import java.lang.reflect.Method;
10-
import java.util.regex.Matcher;
11-
126
/**
137
* @author Anthony Eden
148
*/
@@ -93,43 +87,16 @@ public boolean equals(Object obj) {
9387
if ((obj == null) || (obj.getClass() != this.getClass())) {
9488
return false;
9589
}
96-
// object must be Permissions at this point
90+
if (obj == this) {
91+
return true;
92+
}
9793
Permissions test = (Permissions) obj;
98-
Class cl = this.getClass();
99-
Method[] method = cl.getMethods();
100-
for (int i = 0; i < method.length; i++) {
101-
Matcher m = StringUtilities.getterPattern.matcher(method[i].getName());
102-
if (m.find() && !method[i].getName().equals("getClass")) {
103-
try {
104-
Object res = method[i].invoke(this, null);
105-
Object resTest = method[i].invoke(test, null);
106-
String retType = method[i].getReturnType().toString();
107-
if (retType.indexOf("class") == 0) {
108-
if (res != null && resTest != null) {
109-
if (!res.equals(resTest))
110-
return false;
111-
} else {
112-
// return false;
113-
}
114-
} else if (retType.equals("int")) {
115-
if (!((Integer) res).equals(((Integer) resTest)))
116-
return false;
117-
} else if (retType.equals("boolean")) {
118-
if (!((Boolean) res).equals(((Boolean) resTest)))
119-
return false;
120-
} else {
121-
System.out.println(method[i].getName() + "|" + method[i].getReturnType().toString());
122-
}
123-
} catch (IllegalAccessException ex) {
124-
System.out.println("equals " + method[i].getName() + " " + ex);
125-
} catch (InvocationTargetException ex) {
126-
// System.out.println("equals " + method[i].getName() + " " + ex);
127-
} catch (Exception ex) {
128-
System.out.println("equals " + method[i].getName() + " " + ex);
129-
}
130-
}
94+
// id seems to be photo id
95+
if (id == null ? test.id == null : id.equals(test.id)) {
96+
return publicFlag == test.publicFlag && friendFlag == test.friendFlag && familyFlag == test.familyFlag && comment == test.comment
97+
&& addmeta == test.addmeta;
13198
}
132-
return true;
99+
return false;
133100
}
134101

135102
@Override

0 commit comments

Comments
 (0)