StackOverflow has a good discussion about this exact topic in this Q&A. In the top rated question, kronoz notes:
Returning null is usually the best idea if you intend to indicate that no data is available.
An empty object implies data has been returned, whereas returning null clearly indicates that nothing has been returned.
Additionally, returning a null will result in a null exception if you attempt to access members in the object, which can be useful for highlighting buggy code - attempting to access a member of nothing makes no sense. Accessing members of an empty object will not fail meaning bugs can go undiscovered.
Personally, I like to return empty strings for functions that return strings to minimize the amount of error handling that needs to be put in place. However, you'll need to make sure that the group that your working with will follow the same convention - otherwise the benefits of this decision won't be achieved.
However, as the poster in the SO answer noted, nulls should probably be returned if an object is expected so that there is no doubt about whether data is being returned.
In the end, there's no single best way of doing things. Building a team consensus will ultimately drive your team's best practices.
Answer from JW8 on Stack ExchangeStackOverflow has a good discussion about this exact topic in this Q&A. In the top rated question, kronoz notes:
Returning null is usually the best idea if you intend to indicate that no data is available.
An empty object implies data has been returned, whereas returning null clearly indicates that nothing has been returned.
Additionally, returning a null will result in a null exception if you attempt to access members in the object, which can be useful for highlighting buggy code - attempting to access a member of nothing makes no sense. Accessing members of an empty object will not fail meaning bugs can go undiscovered.
Personally, I like to return empty strings for functions that return strings to minimize the amount of error handling that needs to be put in place. However, you'll need to make sure that the group that your working with will follow the same convention - otherwise the benefits of this decision won't be achieved.
However, as the poster in the SO answer noted, nulls should probably be returned if an object is expected so that there is no doubt about whether data is being returned.
In the end, there's no single best way of doing things. Building a team consensus will ultimately drive your team's best practices.
In all the code I write, I avoid returning null from a function. I read that in Clean Code.
The problem with using null is that the person using the interface doesn't know if null is a possible outcome, and whether they have to check for it, because there's no not null reference type.
In F# you can return an option type, which can be some(Person) or none, so it's obvious to the caller that they have to check.
The analogous C# (anti-)pattern is the Try... method:
public bool TryFindPerson(int personId, out Person result);
Now I know people have said they hate the Try... pattern because having an output parameter breaks the ideas of a pure function, but it's really no different than:
class FindResult<T>
{
public FindResult(bool found, T result)
{
this.Found = found;
this.Result = result;
}
public bool Found { get; private set; }
// Only valid if Found is true
public T Result { get; private set;
}
public FindResult<Person> FindPerson(int personId);
...and to be honest you can assume that every .NET programmer knows about the Try... pattern because it's used internally by the .NET framework. That means they don't have to read the documentation to understand what it does, which is more important to me than sticking to some purist's view of functions (understanding that result is an out parameter, not a ref parameter).
So I'd go with TryFindPerson because you seem to indicate it's perfectly normal to be unable to find it.
If, on the other hand, there's no logical reason that the caller would ever provide a personId that didn't exist, I would probably do this:
public Person GetPerson(int personId);
...and then I'd throw an exception if it was invalid. The Get... prefix implies that the caller knows it should succeed.
A solution such as Option<T> is not a so bad idea. In fact, it has been successfully introduced in Haskell as a Maybe (but I don't know if this is the first time this solution has been used). Maybe monads are common in Haskell programs. Since, other languages have adopted the same system. For instance, Scala has an Option type.
If you don't want to become dependent of Guava, you could use Java 8, which introduces the Optional class.
However, Option/Maybe/… types are more relevant in a functional environment, because what you typically want to do is to get a behaviour/a property from the element if it really exists, and to get nothing or a default behaviour/property otherwise. So, a typical use of Options consists in
- applying a function A->B on the
Option[A]in order to get anOption[B]; - switching your code on the basis of the real content of the Option (for instance, by using Pattern matching if the language supports it and if Option has two subtypes, or by using method overload);
- or filtering the Options that represent (non-)existing elements.
As an alternative, you could use the Null Object Pattern, in which you have an abstract class MyClass and two concrete subclasses: MyRealClass and MyNullClass. You manipulate instances of MyClass, but generate instances of MyRealClass (if the element is really existing) or MyNullClass (if the element doesn't exist). MyNullClass contains the default behaviours/properties. If the null objects are stateless (which is typically the case), one could cache them, or even make them singletons.
This pattern is described in [Fowler].
[Fowler] Martin Fowler, Kent Beck, Refactoring: Improving the Design of Existing Code.
The whole point of a value object is that equality isn't based on identity; you could very well have two distinct blank objects. In fact, whether all blank objects are the same or different objects should be an implementation detail. For those reasons, the second approach is not good. If you look at the Java 8 APIs, there's a notion of a "value-based class" with the following properties:
- are final and immutable (though may contain references to mutable objects);
- have implementations of equals, hashCode, and toString which are computed solely from the instance's state and not from its identity or the state of any other object or variable;
- make no use of identity-sensitive operations such as reference equality (==) between instances, identity hash code of instances, or synchronization on an instances's intrinsic lock;
- are considered equal solely based on equals(), not based on reference equality (==);
- do not have accessible constructors, but are instead instantiated through factory methods which make no committment as to the identity of returned instances;
- are freely substitutable when equal, meaning that interchanging any two instances x and y that are equal according to equals() in any computation or method invocation should produce no visible change in behavior.
The documentation further adds:
A program may produce unpredictable results if it attempts to distinguish two references to equal values of a value-based class, whether directly via reference equality or indirectly via an appeal to synchronization, identity hashing, serialization, or any other identity-sensitive mechanism. Use of such identity-sensitive operations on instances of value-based classes may have unpredictable effects and should be avoided.
There's no way to disable the == operator in Java so that warning is the best you can do.
Thus, your first approach is correct, with the caveat that foo1.equals(foo2) should always be true when foo1.isBlank() and foo2.isBlank() even if foo1 != foo2.
You can't do it directly, you should provide your own way to check this. Eg.
class MyClass {
Object attr1, attr2, attr3;
public boolean isValid() {
return attr1 != null && attr2 != null && attr3 != null;
}
}
Or make all fields final and initialize them in constructors so that you can be sure that everything is initialized.
import org.apache.commons.lang3.ObjectUtils;
if(ObjectUtils.isEmpty(yourObject)){
//your block here
}
I was also required to produce such a structure for legacy client compatibility, here is my solution (depends on Spring Boot since uses @JsonComponent annotation)
Create "special object" that will be treated as empty
public class EmptyObject {
}
Create property in your model
@JsonProperty("data")
private EmptyObject data = new EmptyObject();
public EmptyObject getData() {
return data;
}
Create serializer that will process empty object above
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.ser.std.StdSerializer;
import com.sevensenders.datahub.api.service.response.model.EmptyObject;
import org.springframework.boot.jackson.JsonComponent;
import java.io.IOException;
@JsonComponent
public class EmptyObjectSerializer extends StdSerializer<EmptyObject> {
public EmptyObjectSerializer() {
this(null);
}
public EmptyObjectSerializer(Class<EmptyObject> t) {
super(t);
}
@Override
public void serialize(EmptyObject value, JsonGenerator gen, SerializerProvider provider) throws IOException {
// to maintain AF compatible format it is required to write {} instead of null
gen.writeStartObject();
gen.writeEndObject();
}
}
Output:
{
...
"data": {}
}
If you don't want to write your own serializer you can use this approach of declaring type of field as ObjectNode:
private ObjectNode data;
You can set/initialize it like this:
data = new ObjectNode(JsonNodeFactory.instance)
There is a way to initialize the destination object in Dozer.
- Define a factory class for the destination Object.
- Specify the create-method for the destination object as the method in the above factory class.
Take your example, I presume you want to map Person to PersonVO and Address to AddressVO.
Create a factory class for PersonVO
public abstract class PersonVOFactory {
private PersonVOFactory() {
}
public static PersonVO createPersonVO() {
PersonVO personVO = new PersonVO();
personVO.setAddressVO(new AddressVO());
return personVO;
}
}
Specify the above create method for Person to PersonVO mapping.
<mapping map-id="person-mapping">
<class-a>Person</class-a>
<class-b create-method="PersonVOFactory.createPersonVO">PersonVO</class-b>
<field>
<a>address.addressField1</a>
<b>addressVO.addressField1</b>
</field>
<field>
<a>address.addressField2</a>
<b>addressVO.addressField2</b>
</field>
</mapping>
Now the statement, personvo.getAddressVO.getAddressField1() will not throw an NPE.
If you can modify the setter method of the PersonVO class, you could do as follow :
public void setAddressVO(AddressVO address) {
this.addressVO = address == null ? new AddressVO() : address;
}
Thus you will be sure that the field addressVO is never null.
Please note that this is rather dirty because :
- if a person has no address, his address field should be
nullrather than an emptyAddressVOobject. - a customized setter like
this is a bad practice as most users of your class would expect the
set method to set the value and do nothing more. You can optionally
create a new method
initAddressVOand call this method from dozer using theset-methodattribute in the configuration file.
It would be much better (if it's possible) to check if address is null with some if to avoid the NullPointerException.
An "empty object" is pretty ambiguous in java terms. I could interpret that as this:
Object empty = new Object();
which is about the emptiest object you can create.
However in your example,
Name myName = new Name();
That's going to create an object based on whatever code you've put in your default constructor. (Which, i guess if you're setting everything to a default value, is pretty empty)
If Name has a parameterless constructor, sure. Whether or not it's "empty" depends on what that constructor does or what defaults it may have.
How do you define "empty object" anyway?
For example, if you want a variable but don't want it to actually have an object, you can just declare the variable without initializing it:
Name myName;
In this case myName will be null (or "unassigned"? depends on the context), but will be of type Name and can be used as such later (once it's assigned a value).
All the variable itself does is point to a location in memory where the "object" exists. So something like Name myName doesn't "create" an object, it just creates the pointer to a memory location. new Name() actually creates an object by calling its constructor. When used together like in your example, the latter half creates the object and then the former half points to the location in memory where the object exists.
Can't you just replace {} with NULL before passing it to the GSON?
Make your TypeAdapter<String>'s read method like this:
public String read(JsonReader reader) throws IOException {
boolean nextNull = false;
while (reader.peek() == JsonToken.BEGIN_ARRAY || reader.peek() == JsonToken.END_ARRAY) {
reader.skipValue();
nextNull = true;
}
return nextNull ? null : reader.nextString();
}
Explain: when next token is [ or ], just skip it and return null.
If you replace all [] to null use String#replaceAll directly, some real string may be replaced as well, This may cause some other bugs.
If you return an empty collection (but not necessarily Collections.emptyList()), you avoid surprising downstream consumers of this method with an unintentional NPE.
This is preferable to returning null because:
- The consumer doesn't have to guard against it
- The consumer can operate on the collection irrespective of how many elements are in it
I say not necessarily Collections.emptyList() since, as you point out, you're trading one runtime exception for another in that adding to this list will be unsupported and once again surprise the consumer.
The most ideal solution to this: eager initialization of the field.
private List<String> bone = new ArrayList<>();
The next solution to this: make it return an Optional and do something in case it doesn't exist. Instead of throwing you could also provide the empty collection here if you so desired.
Dog dog = new Dog();
dog.get().orElseThrow(new IllegalStateException("Dog has no bones??"));
Because the alternative to returning an empty collection is generally returning null; and then callers have to add guards against NullPointerException. If you return an empty collection that class of error is mitigated. In Java 8+ there is also an Optional type, which can serve the same purpose without a Collection.
My company uses files that are essentially merged XML files into one. I wrote a pretty basic parser and node object that contains the tag name, text, and child node objects. Recently I ran into an error where the file couldn't be found, and my function that searches for child nodes returned null (as it should). But it caused a runtime error, so I'm trying to figure out the best way to address issue. Should I return an empty object so that the code doesn't crash, or should I continue returning null and wrap it in a try/catch?
Returning null is usually the best idea if you intend to indicate that no data is available.
An empty object implies data has been returned, whereas returning null clearly indicates that nothing has been returned.
Additionally, returning a null will result in a null exception if you attempt to access members in the object, which can be useful for highlighting buggy code - attempting to access a member of nothing makes no sense. Accessing members of an empty object will not fail meaning bugs can go undiscovered.
It depends on what makes the most sense for your case.
Does it make sense to return null, e.g. "no such user exists"?
Or does it make sense to create a default user? This makes the most sense when you can safely assume that if a user DOESN'T exist, the calling code intends for one to exist when they ask for it.
Or does it make sense to throw an exception (a la "FileNotFound") if the calling code is demanding a user with an invalid ID?
However - from a separation of concerns/SRP standpoint, the first two are more correct. And technically the first is the most correct (but only by a hair) - GetUserById should only be responsible for one thing - getting the user. Handling its own "user does not exist" case by returning something else could be a violation of SRP. Separating into a different check - bool DoesUserExist(id) would be appropriate if you do choose to throw an exception.
Based on extensive comments below: if this is an API-level design question, this method could be analogous to "OpenFile" or "ReadEntireFile". We are "opening" a user from some repository and hydrating the object from the resultant data. An exception could be appropriate in this case. It might not be, but it could be.
All approaches are acceptable - it just depends, based on the larger context of the API/application.
MattPutnam's answer is right on point, and I second it. I'd add this: the concept of "null object," when you analyze it, seems to boil down to the mathematical concept of a monoid. You can think of it this way: a monoid is a type that has both of these things:
- An "append," "sum" or similar operation, which needs to be associative:
a.op(b).op(c)is the same asa.op(b.op(c)). - An "empty," "zero" or "null" value, that acts as the neutral element or identity element of the operation.
The classic example of the null object pattern is to return an empty list or array instead of null. Well, lists are a monoid, with append as the operation and the empty list as the neutral element.
Now, the problem that you face in your Car example is that Car isn't really a monoid; there is no notion of "the empty car" or "the neutral car", and there isn't really a sensible operation that you could use to combine two Cars into one.
So the recommendation you're rightly getting is to use something like the Java 8 Optional. And the trick is that no matter what type T is, Optional<T> is a monoid:
- The monoid's "combine" operation is "pick the first value if it's not
empty, otherwise pick the second value":x || empty = xempty || x = x
- The neutral element is
Optional.empty(), becauseOptional.empty().orElse(anything)is the same as justanything.
So basically, Optional<T> is a wrapper that adds a null object to types like Car that don't have one. The Optional<T>.orElse(T value) method that is a slightly refactored version of the "pick first non-empty value" monoid.
The null object pattern only makes sense when there's a reasonable, functional value for the null object to be. The purpose isn't to defer null, as you've described, but to completely eliminate the idea of null by representing the nothingness or emptiness with an actual piece of data that is still functional. For example, the natural case of holes in a tree structure, as described in the Wikipedia article.
A null car doesn't make sense. In this case, it seems like the more appropriate thing would be for getCar() to return Optional<Car>.
Method overloading can make your implementations more efficient and cleaner:
public static boolean isEmpty(Collection obj) {
return obj == null || obj.isEmpty();
}
public static boolean isEmpty(String string) {
return string == null || string.trim().isEmpty();
}
public static boolean isEmpty(Object obj) {
return obj == null || obj.toString().trim().isEmpty();
}
The Collection version is as efficient as possible.
The String version would be more efficient without the trimming. It would be best to trim your strings as soon you see them, long before they reach this call. If you can review the callers and make sure that the strings are always trimmed at their origins, then you can remove .trim() for best performance.
The Object version can be inefficient, depending on the toString implementation of the objects that will be passed to it, and because of the trimming.
I removed the comparison with null from there, because it seems pointless to me. I mean, a class whose toString method says "null" would seem very very odd.
In any case, you don't really want the Object version to be called, at all. Most importantly because it probably won't even work. Take for example an empty Map. Its toString method returns the string {}, which won't match your conditions of emptiness. (For this type you should definitely add isEmpty(Map<?, ?> map) to benefit from its isEmpty method.)
If performance is so critical, then add more overloaded implementations for all other types that you care about, for example:
public static boolean isEmpty(Something obj) {
return obj == null || obj.isEmpty();
}
Finally, especially when something is so important, you definitely want to unit test it, for example:
@Test
public void testEmptyObject() {
assertTrue(isEmpty((Object) null));
assertFalse(isEmpty(new Object()));
}
@Test
public void testEmptyString() {
assertFalse(isEmpty("hello"));
assertTrue(isEmpty(""));
assertTrue(isEmpty(" "));
assertTrue(isEmpty((Object) null));
}
@Test
public void testEmptySet() {
assertFalse(isEmpty(new HashSet<String>(Arrays.asList("hello"))));
assertTrue(isEmpty(new HashSet<String>()));
}
@Test
public void testEmptyMap() {
Map<String, String> map = new HashMap<String, String>();
assertTrue(isEmpty(map));
map.put("hello", "hi");
assertFalse(isEmpty(map));
}
Don't.
I mean. Don't use the same method for all kinds of objects.
This method does not make much sense to me.
This line smells. A lot.
if (obj instanceof Collection)
return ((Collection<?>) obj).size() == 0;
Beware of instanceof operator.
I am sure that whatever it is that you are trying to do here, there are better ways to do it.
Java is a statically typed language, use the static types whenever possible. If you really don't know what type the object have, then I will provide another alternative below.
// is below line expensive?
final String s = String.valueOf(obj).trim();
That depends, on the implementation of the object's toString method.
The implementation of String.valueOf is:
public static String valueOf(Object obj) {
return (obj == null) ? "null" : obj.toString();
}
return s.length() == 0 || s.equalsIgnoreCase("null");
You have already checked for obj == null. The string will only be null when the object's toString method makes it so. And instead of s.length() == 0 you can use s.isEmpty() directly. (Although that is implemented as string length == 0
Do it differently
If possible, have the types of objects you're investigating implement an interface that provides an isEmpty method and let the object decide for itself if it is empty or not.
If that is not possible, you can use a dynamically created map with ways to determine whether or not the object is "empty".
Map<Class<?>, EmptyChecker> map = new HashMap<>();
map.put(String.class, new StringEmptyChecker());
map.put(Point.class, new PointEmptyChecker());
This is a kind of Strategy pattern.
Then to determine if an object is empty:
EmptyChecker checker = map.get(obj.getClass());
checker.isEmpty(obj);
The whole thing is kinda weird though, I can't really see a particular use-case for this kind of method.