patternjavaMinor
Checking for existence of object by id, name, display name, or web id
Viewed 0 times
existencecheckingdisplaynameforwebobject
Problem
I have the following code:
My
I would like to reduce this chunk of code, if possible, because I have many objects that require the
A possible solution I thought of would be to do this:
and then call it with
One of the answers I got over was this, from user
You are recognizing that the "does exist" part of all of these methods is exactly the same, and that makes you want to avoid repeating it, but the "ByXxx" part of these methods is completely different.
What you've got is far better than what you're thinking of doing. Please don't change your method signature to require callers to provide null values for all but one argument. That is highly error-prone, as it provides no compile-time errors for a variety of different ways that people might use the method signature incorrectly.
One thing you might want to consider doing is separating the "does exist" piece of this into its own mechanism:
```
public final Optional byId(Long id) {
return Optional.ofNullable(dataAcce
public final boolean doesExistById(Long id) {
return dataAccessObject.findById(id) != null;
}
public final boolean doesExistByName(String name) {
return dataAccessObject.findByName(name) != null;
}
public final boolean doesExistByDisplayName(String displayName) {
return dataAccessObject.findByDisplayName(displayName) != null;
}
public final boolean doesExistByWebId(String webId) {
return dataAccessObject.findByWebId(webId) != null;
}My
Product class has properties id, name, displayName, wedId.dataAccessObject.findBy____() returns an object of type Product, if it can be found in the data store, or null if it cannot.I would like to reduce this chunk of code, if possible, because I have many objects that require the
doesExist() pattern as above. The client code will only know one of these properties.A possible solution I thought of would be to do this:
public final boolean doesExist(Long id, String name, String displayName, String webId) {..}and then call it with
null for unknown fields while using if statements to determine which field has a value. But is there another way that is more elegant?One of the answers I got over was this, from user
StriplingWarrior:You are recognizing that the "does exist" part of all of these methods is exactly the same, and that makes you want to avoid repeating it, but the "ByXxx" part of these methods is completely different.
What you've got is far better than what you're thinking of doing. Please don't change your method signature to require callers to provide null values for all but one argument. That is highly error-prone, as it provides no compile-time errors for a variety of different ways that people might use the method signature incorrectly.
One thing you might want to consider doing is separating the "does exist" piece of this into its own mechanism:
```
public final Optional byId(Long id) {
return Optional.ofNullable(dataAcce
Solution
To me such a method
is already a code smell. What do you expect it to be used for. Something like
which means you're doing the lookup twice in the first branch. This is not too bad as there's caching somewhere, but it's strange. So I'd prefer to have no such method.
The other thing is that all it does id delegating to the
you call
Does it make sense? It might, but without more code we can't tell.
It looks like a typical XY problem. You're asking how to improve X (the repetitive
So all we can do is to tell you what not to do.
This could be a good idea if you needed a combined search often. But you're speaking about many methods and converting them into one method with many arguments is a step obviously in the wrong direction. It's much longer on the call site, much more error-prone and slightly less efficient. No gain.
My favorite rant says it all, though in this case it mightn't be that bad. You could use something like
It wouldn't reduce the number of needed methods, but it could make them exactly what you need (remember, Y is kept secret).
With Java 8, it could get pretty compact.
public final boolean doesExistById(Long id) {
return dataAccessObject.findById(id) != null;
}is already a code smell. What do you expect it to be used for. Something like
if (doesExistById(id)) {
return getById(id);
} else {
return new Something(id);
}which means you're doing the lookup twice in the first branch. This is not too bad as there's caching somewhere, but it's strange. So I'd prefer to have no such method.
The other thing is that all it does id delegating to the
dataAccessObject. Can't you use it directly. It looks like instead of callingdataAccessObject.findById(id)you call
somethingContainingTheDataAccessObject.findById(id)Does it make sense? It might, but without more code we can't tell.
It looks like a typical XY problem. You're asking how to improve X (the repetitive
doesExist* while having a problem Y (kept secret).So all we can do is to tell you what not to do.
public final boolean doesExist(Long id, String name, String displayName, String webId) {..}This could be a good idea if you needed a combined search often. But you're speaking about many methods and converting them into one method with many arguments is a step obviously in the wrong direction. It's much longer on the call site, much more error-prone and slightly less efficient. No gain.
public final Optional byId(Long id) {
return Optional.ofNullable(dataAccessObject.findById(id));
}My favorite rant says it all, though in this case it mightn't be that bad. You could use something like
public final Something getOrCreateById(Long id, Function creator) {
Something result = dataAccessObject.findById(id);
return result != null ? result : creator.apply(id);
}It wouldn't reduce the number of needed methods, but it could make them exactly what you need (remember, Y is kept secret).
With Java 8, it could get pretty compact.
Code Snippets
public final boolean doesExistById(Long id) {
return dataAccessObject.findById(id) != null;
}if (doesExistById(id)) {
return getById(id);
} else {
return new Something(id);
}dataAccessObject.findById(id)somethingContainingTheDataAccessObject.findById(id)public final boolean doesExist(Long id, String name, String displayName, String webId) {..}Context
StackExchange Code Review Q#96771, answer score: 6
Revisions (0)
No revisions yet.