patternjavaMinor
Detecting change of a name or a part
Viewed 0 times
changepartnamedetecting
Problem
We have a fair amount of code which looks like this:
That is, if the thing's current part is null and we're trying to set its name to null, that's not a change. If the part is null or the name is null, but not both, that is a change. And if neither is null, we have to look at it a little closer.
Thing and Part represent different entities for different such methods, so the first and final lines have the same general form but different specifics.
As written, this generates a SimplifiableIfStatement warning in Intellij, which would prefer to see it as:
Which is arguably simpler, but certainly not easier for me to read. I can of course suppress the warning with
I would like to find a way to extract this bit:
which is duplicated in all the instances of this code, because (a) it triggers the SimplifiableIfStatement warning and (b) it just seems too wordy to have it everywhere. So far I've come up with this:
```
private Boolean changedFromNull(Object a, Object b) {
// if both are null, nothing has changed.
if (a == null && b == null) {
return false;
}
// if only one is null, something has changed
if (a == null || b == null) {
return true;
}
// if neither is null, further analysis is required
private boolean isThingChanged(String newName, Thing thing) {
final Part part = thing.getPart();
if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}
return !part.getName().equals(newName);
}That is, if the thing's current part is null and we're trying to set its name to null, that's not a change. If the part is null or the name is null, but not both, that is a change. And if neither is null, we have to look at it a little closer.
Thing and Part represent different entities for different such methods, so the first and final lines have the same general form but different specifics.
As written, this generates a SimplifiableIfStatement warning in Intellij, which would prefer to see it as:
private boolean isThingChanged(String newName, Thing thing) {
final Part part = thing.getPart();
return !(part == null && newName == null) && (part == null || newName == null || !part.getName().equals(newName));
}Which is arguably simpler, but certainly not easier for me to read. I can of course suppress the warning with
@SuppressWarnings("SimplifiableIfStatement") but prefer to avoid that if possible.I would like to find a way to extract this bit:
if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}which is duplicated in all the instances of this code, because (a) it triggers the SimplifiableIfStatement warning and (b) it just seems too wordy to have it everywhere. So far I've come up with this:
```
private Boolean changedFromNull(Object a, Object b) {
// if both are null, nothing has changed.
if (a == null && b == null) {
return false;
}
// if only one is null, something has changed
if (a == null || b == null) {
return true;
}
// if neither is null, further analysis is required
Solution
A good option is to use the
Note also, that instead of doing if-conditions and returning true, or false, you can do a direct return of the condition... For example, the
And those can be simplified to just:
Note also, that your code runs the risk of a NullPointerException if
Your code:
Could be:
Writing that as a ternary is an option:
Objects utility class (available since Java 7)Note also, that instead of doing if-conditions and returning true, or false, you can do a direct return of the condition... For example, the
part == null is common to these two expressions:if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}And those can be simplified to just:
if (part == null) {
return newName != null;
}Note also, that your code runs the risk of a NullPointerException if
part.getName() returns null. Is that possible?Your code:
private boolean isThingChanged(String newName, Thing thing) {
final Part part = thing.getPart();
if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}
return !part.getName().equals(newName);
}Could be:
private boolean isThingChanged(String newName, Thing thing) {
if (thing.getPart() == null) {
return newName != null;
}
return !Objects.equal(thing.getPart().getName(), newName);
}Writing that as a ternary is an option:
private boolean isThingChanged(String newName, Thing thing) {
return thing.getPart() == null ? newName != null : !Objects.equal(thing.getPart().getName(), newName);
}Code Snippets
if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}if (part == null) {
return newName != null;
}private boolean isThingChanged(String newName, Thing thing) {
final Part part = thing.getPart();
if (part == null && newName == null) {
return false;
}
if (part == null || newName == null) {
return true;
}
return !part.getName().equals(newName);
}private boolean isThingChanged(String newName, Thing thing) {
if (thing.getPart() == null) {
return newName != null;
}
return !Objects.equal(thing.getPart().getName(), newName);
}private boolean isThingChanged(String newName, Thing thing) {
return thing.getPart() == null ? newName != null : !Objects.equal(thing.getPart().getName(), newName);
}Context
StackExchange Code Review Q#71706, answer score: 4
Revisions (0)
No revisions yet.