ChatGPT解决这个技术问题 Extra ChatGPT

Is returning null bad design? [closed]

Closed. This question is opinion-based. It is not currently accepting answers. Want to improve this question? Update the question so it can be answered with facts and citations by editing this post. Closed 3 years ago. Improve this question

I've heard some voices saying that checking for a returned null value from methods is bad design. I would like to hear some reasons for this.

pseudocode:

variable x = object.method()
if (x is null) do something
Elaborate: where are these people who say it's bad? Links?
If the method is something you have control over, you can have unit tests to make sure it never returns null, otherwise, I don't see why it would be a bad practice to check if it's null, after that call; it maybe a bad practice on that method to return null, but you have to protect your code
Raising exceptions just because there is no data to be returned is incredibly annoying. Normal program flow should not throw exceptions.
@David: That's what I said really. If a method should return data, but there is none, that means something also went wrong. That's not normal program flow :)
@Thorarin: "normal" program flow is quite a stretchable concept: not really a solid basis for an argument.

A
Adamski

The rationale behind not returning null is that you do not have to check for it and hence your code does not need to follow a different path based on the return value. You might want to check out the Null Object Pattern which provides more information on this.

For example, if I were to define a method in Java that returned a Collection I would typically prefer to return an empty collection (i.e. Collections.emptyList()) rather than null as it means my client code is cleaner; e.g.

Collection<? extends Item> c = getItems(); // Will never return null.

for (Item item : c) { // Will not enter the loop if c is empty.
  // Process item.
}

... which is cleaner than:

Collection<? extends Item> c = getItems(); // Could potentially return null.

// Two possible code paths now so harder to test.
if (c != null) {
  for (Item item : c) {
    // Process item.
  }
}

Yes, way better than returning null and hoping the client remembers to deal with the case.
I'll happily agree that returning null is insane when it's used as a substitute for empty containers (or strings). That's not the common case though.
+1 for the Null Object Pattern for me too. Also when I actually want to return null, I have been naming the method like getCustomerOrNull() to make it explicit. I think a method is named well when the reader doesn't want to look at the implementation.
The comment '// Two possible code paths now' is not correct; you have two code paths in either way. However, with the null Collection object in your first example, the 'null' code path is shorter. You still have two paths though, and you still need to test two paths.
@MSalters - arguably every variable in OO languages with null is a "container", which contains zero or one pointers to objects. (That's certainly how it's explicitly modelled by Haskell's Maybe in those cases, and is all the better for doing so.)
M
Max Chernyak

Here's the reason.

In Clean Code by Robert Martin he writes that returning null is bad design when you can instead return, say, empty array. Since expected result is an array, why not? It'll enable you to iterate over result without any extra conditions. If it's an integer, maybe 0 will suffice, if it's a hash, empty hash. etc.

The premise is to not force calling code to immediately handle issues. Calling code may not want to concern itself with them. That's also why in many cases exceptions is better than nil.


This is the Null Object Pattern mentioned in this answer: stackoverflow.com/questions/1274792/…
The idea here is that on error, you return an empty/blank version of whatever object type you would return normally. An empty array or string would work for those cases, for instance. Returning "NULL" is appropriate when you would normally return a pointer (since NULL is essentially an empty pointer). Returning NULL on error from a function that normally returns, say, a hash can be confusing. Some languages handle this better than others but in general, consistency is the best practice.
You don't necessarily return on error, unless the error is somehow controlling the flow (which is not a good practice) or abstracted in API or interface. Errors are there to propagate to any level on which you decide to catch it, hence you don't have to deal with it in calling context. They are by default null-object-pattern-friendly.
Unfortunately, calling code doesn't always consider the case of an empty collection (much like it doesn't consider the null case). That may or may not be a problem.
Z
ZeroConcept

Good uses of returning null:

If null is a valid functional result, for example: FindFirstObjectThatNeedsProcessing() can return null if not found and the caller should check accordingly.

Bad uses: Trying to replace or hide exceptional situations such as:

catch(...) and return null

API dependency initialization failed

Out of disk space

Invalid input parameters (programming error, inputs must be sanitized by the caller)

etc

In those cases throwing an exception is more adequate since:

A null return value provides no meaningful error info

The immediate caller most likely cannot handle the error condition

There is no guarantee that the caller is checking for null results

However, Exceptions should not be used to handle normal program operation conditions such as:

Invalid username/password (or any user-provided inputs)

Breaking loops or as non-local gotos


"Invalid username" seems like a good cause for an Exception, though.
Users entering invalid login/passwords should not be treated as an exceptional conditions, its part of normal program operation. However, the authentication system failing to respond (e.g. active directory) is an exceptional condition.
I'd say it depends: boolean login(String,String) seems fine and so does AuthenticationContext createAuthContext(String,String) throws AuthenticationException
In your example, if there is no first object that needs processing, then there are two reasonable ways to handle that case. The first is the Null Object pattern. Create a final subclass that represents a non-existent version of the class. It has reasonable defaults and throws exceptions when nonsense actions are requested. The other technique is Option. This is what is available in Java8 and Scala. Either of these show the developers intent. null cannot show intent because it has too many possible meanings. ** Showing Intent** is the most important aspect of code.
y
yegor256

Yes, returning NULL is a terrible design, in object-oriented world. In a nutshell, NULL usage leads to:

ad-hoc error handling (instead of exceptions)

ambiguous semantic

slow instead of fast failing

computer thinking instead of object thinking

mutable and incomplete objects

Check this blog post for a detailed explanation: http://www.yegor256.com/2014/05/13/why-null-is-bad.html. More in my book Elegant Objects, Section 4.1.


I agree strongly. I don't even like the null object pattern which appears to be a 'whitewash' delay tactic. Either your method is suppose to always succeed or it may not. If it should always succeed, then throw. If it may not succeed, then design the method so the consumer knows, e.g. bool TryGetBlah(out blah) or FirstOrNull() or MatchOrFallback(T fallbackValue).
I too do not like return null. You are separating root cause from symptom, making debugging harder. Either throw an exception (fail fast), or call a checker method that returns a boolean first (e.g. isEmpty()) and only if it is true then call the method. People argue against the 2nd one say it's poorer performance - but as Unix Philosophy says, "value human time over machine time" (i.e. trivially slower performance wastes less time than developers debugging code that is giving spurious errors).
B
Brandon

Who says this is bad design?

Checking for nulls is a common practice, even encouraged, otherwise you run the risk of NullReferenceExceptions everywhere. Its better to handle the error gracefully than throw exceptions when you don't need to.


+1. Code that throws exceptions for problems that can be mitigated on the spot makes me sad.
How sad do you get when coders forget to check for nulls and then get mysterious null pointer exceptions? Checked exceptions, where the compiler reminds the user they didn't deal with error conditions avoid taht class of coding errors.
@djna: I guess this is also sad but I've found that the same type of coders that "forget" to check for nulls are the ones who when dealing with checked exceptions frequently end up swallowing them.
It's easier to spot the presence of an empty catch block than the absence of a null-check.
@Preston: I strongly disagree. Absence of a null-check you'll spot immediately, when it crashes. Swallowed exceptions can propogate mysterious and subtle errors for years...
j
jcollum

Based on what you've said so far, I think there's not enough information.

Returning null from a CreateWidget()method seems bad.

Returning null from a FindFooInBar() method seems fine.


Similar to my convention: Single item queries - Create... returns a new instance, or throws; Get... returns an expected existing instance, or throws; GetOrCreate... returns an existing instance, or new instance if none exists, or throws; Find... returns an existing instance, if it exists, or null. For collection queries - Get... always returns a collection, which is empty if no matching items are found.
NULL's bad for reasons given in yegor256 & hakuinin's answer, returning a valid but empty object simplifies reasoning overal
B
Bahadır Yağan

Its inventor says it is a billion dollar mistake!


G
Greg Beech

It depends on the language you're using. If you're in a language like C# where the idiomatic way of indicating the lack of a value is to return null, then returning null is a good design if you don't have a value. Alternatively, in languages such as Haskell which idiomatically use the Maybe monad for this case, then returning null would be a bad design (if it were even possible).


+1 for mentioning maybe monad. I find the definition of null in languages such as C# and Java are often overloaded and given some meaning in the domain. If you look up the null keyword in the language specs, it simply means "an invalid pointer". That likely means nothing in any problem domain.
Problem is you don't know if it's a "missing value" null or "not initialized" null
B
Boris Callens

If you read all the answers it becomes clear the answer to this question depends on the kind of method.

Firstly, when something exceptional happens (IOproblem etc), logically exceptions are thrown. When exactly something is exceptional is probably something for a different topic..

Whenever a method is expected to possibly have no results there are two categories:

If it is possible to return a neutral value, do so. Empty enumrables, strings etc are good examples

If such a neutral value does not exist, null should be returned. As mentioned, the method is assumed to possibly have no result, so it is not exceptional, hence should not throw an exception. A neutral value is not possible (for example: 0 is not especially a neutral result, depending on the program)

Untill we have an official way to denote that a function can or cannot return null, I try to have a naming convention to denote so. Just like you have the TrySomething() convention for methods that are expected to fail, I often name my methods SafeSomething() when the method returns a neutral result instead of null.

I'm not fully ok with the name yet, but couldn't come up with anything better. So I'm running with that for now.


J
Johann Gerell

I have a convention in this area that served me well

For single item queries:

Create... returns a new instance, or throws

Get... returns an expected existing instance, or throws

GetOrCreate... returns an existing instance, or new instance if none exists, or throws

Find... returns an existing instance, if it exists, or null

For collection queries:

Get... always returns a collection, which is empty if no matching[1] items are found

[1] given some criteria, explicit or implicit, given in the function name or as parameters.


Why GetOne and FindOne return different if not found?
I described the difference. I use Get when I expect it to be there, so that if it's not there, then it's an error and I throw - I never need to check the return value. I use Find if I really don't know if it's there or not - then I need to check the return value.
M
Michael Burr

It's not necessarily a bad design - as with so many design decisions, it depends.

If the result of the method is something that would not have a good result in normal use, returning null is fine:

object x = GetObjectFromCache();   // return null if it's not in the cache

If there really should always be a non-null result, then it might be better to throw an exception:

try {
   Controller c = GetController();    // the controller object is central to 
                                      //   the application. If we don't get one, 
                                      //   we're fubar

   // it's likely that it's OK to not have the try/catch since you won't 
   // be able to really handle the problem here
}
catch /* ... */ {
}

Though you can log a diagnostic error right there, then maybe rethrow the exception. Sometime First Failure Data Capture can be very helpful.
Or wrap the exception in a RuntimeException with diagnostic information in the message string. Keep the information together.
Now (in 2018) I can no longer agree and in such cases we should favor returning Optional<>
佚名

Exceptions are for exceptional circumstances.

If your function is intended to find an attribute associated with a given object, and that object does has no such attribute, it may be appropriate to return null. If the object does not exist, throwing an exception may be more appropriate. If the function is meant to return a list of attributes, and there are none to return, returning an empty list makes sense - you're returning all zero attributes.


If you try to use an attribute with no value, that merits an exception. (I'm assuming your spec says the attribute is optional.) Have a separate method to check whether its value has been set.
S
Steve B.

It's fine to return null if doing so is meaningful in some way:

public String getEmployeeName(int id){ ..}

In a case like this it's meaningful to return null if the id doesn't correspond to an existing entity, as it allows you to distinguish the case where no match was found from a legitimate error.

People may think this is bad because it can be abused as a "special" return value that indicates an error condition, which is not so good, a bit like returning error codes from a function but confusing because the user has to check the return for null, instead of catching the appropriate exceptions, e.g.

public Integer getId(...){
   try{ ... ; return id; }
   catch(Exception e){ return null;}
}

Yup. If you've got an error condition, throw an exception.
This is one case where an exception would be warranted. If you're requesting the name of an employee that doesn't exist, something is obviously going wrong.
I think that's a bit literal, Thorarin-you can quibble with my example, certainly, but I'm sure you can imagine some instance of a function that returns a matching value, or null if there's no match. How about getting a value from a key in a hashtable? (should've thought of that example in the first place).
A hash table returning null for a key that doesn't exist is bad. Doing so automatically means that you can't store null as a value without inconsistent code.
D
Drew Hoskins

For certain scenarios, you want to notice a failure as soon as it happens.

Checking against NULL and not asserting (for programmer errors) or throwing (for user or caller errors) in the failure case can mean that later crashes are harder to track down, because the original odd case wasn't found.

Moreover, ignoring errors can lead to security exploits. Perhaps the null-ness came from the fact that a buffer was overwritten or the like. Now, you are not crashing, which means the exploiter has a chance to execute in your code.


d
djna

What alternatives do you see to returning null?

I see two cases:

findAnItem( id ). What should this do if the item is not found

In this case we could: Return Null or throw a (checked) exception (or maybe create an item and return it)

listItemsMatching (criteria) what should this return if nothing is found?

In this case we could return Null, return an empty list or throw an Exception.

I believe that return null may be less good than the alternatives becasue it requires the client to remember to check for null, programmers forget and code

x = find();
x.getField();  // bang null pointer exception

In Java, throwing a checked exception, RecordNotFoundException, allows the compiler to remind the client to deal with case.

I find that searches returning empty lists can be quite convenient - just populate the display with all the contents of the list, oh it's empty, the code "just works".


Throwing exceptions to indicate a condition which is expected to possibly happen during the normal flow of the program leads to really ugly code, of the sort try { x = find() } catch (RecordNotFound e) { // do stuff }.
Returning empty lists is a good solution but only when the method is in a context that can return lists. For "findById" situations, you need to return null. I don't like RecordNotFound exceptions.
So that's the crux of our differentce in opinion. To my eyes there's not much difference in beauty between x = find(); if ( x = null ) { work } else { do stuff } and try catch. And, if necessary I'm prepared to sacrifice beauty for code correctness. Too often in my life I encounter code where return values have not been checked.
D
David Plumpton

Make them call another method after the fact to figure out if the previous call was null. ;-) Hey, it was good enough for JDBC


D
Denis Biondic

Well, it sure depends of the purpose of the method ... Sometimes, a better choice would be to throw an exception. It all depends from case to case.


V
Vatine

Sometimes, returning NULL is the right thing to do, but specifically when you're dealing with sequences of different sorts (arrays, lists, strings, what-have-you) it is probably better to return a zero-length sequence, as it leads to shorter and hopefully more understandable code, while not taking much more writing on API implementer's part.


G
GregJF

The base idea behind this thread is to program defensively. That is, code against the unexpected. There is an array of different replies:

Adamski suggests looking at Null Object Pattern, with that reply being up voted for that suggestion.

Michael Valenty also suggests a naming convention to tell the developer what may be expected. ZeroConcept suggests a proper use of Exception, if that is the reason for the NULL. And others.

If we make the "rule" that we always want to do defensive programming then we can see that these suggestions are valid.

But we have 2 development scenarios.

Classes "authored" by a developer: The Author

Classes "consumed" by another(maybe) developer: the Developer

Regardless of whether a class returns NULL for methods with a return value or not, the Developer will need to test if the object is valid.

If the developer cannot do this, then that Class/method is not deterministic. That is, if the "method call" to get the object does not do what it "advertises" (eg getEmployee) it has broken the contract.

As an author of a class, I always want to be as kind and defensive ( and deterministic) when creating a method.

So given that either NULL or the NULL OBJECT (eg if(employee as NullEmployee.ISVALID)) needs to be checked and that may need to happen with a collection of Employees, then the null object approach is the better approach.

But I also like Michael Valenty's suggestion of naming the method that MUST return null eg getEmployeeOrNull.

An Author who throws an exception is removing the choice for the developer to test the object's validity, which is very bad on a collection of objects, and forces the developer into exception handling when branching their consuming code.

As a developer consuming the class, I hope the author gives me the ability to avoid or program for the null situation that their class/methods may be faced with.

So as a developer I would program defensively against NULL from a method. If the author has given me a contract that always returns a object (NULL OBJECT always does) and that object has a method/property by which to test the validity of the object, then I would use that method/property to continue using the object, else the object is not valid and I cannot use it.

Bottom line is that the Author of the Class/Methods must provide mechanisms that a Developer can use in their defensive programming. That is, a clearer intention of the method.

The Developer should always use defensive programming to test the validity of the objects returned from another class/method.

regards

GregJF


M
Marcin Deptuła

Other options to this, are: returning some value that indicates success or not (or type of an error), but if you just need boolean value that will indicate success / fail, returning null for failure, and an object for success wouldn't be less correct, then returning true/false and getting the object through parameter. Other approach would to to use exception to indicates failures, but here - there are actually many more voices, that say this is a BAD practice (as using exceptions may be convenient but has many disadvantages). So I personally don't see anything bad in returning null as indication that something went wrong, and checking it later (to actually know if you have succeeded or not). Also, blindly thinking that your method will not return NULL, and then base your code on it, may lead to other, sometimes hard to find, errors (although in most cases it will just crash your system :), as you will reference to 0x00000000 sooner or later).


C
Community

Unintended null functions can arise during the development of a complex programs, and like dead code, such occurrences indicate serious flaws in program structures. A null function or method is often used as the default behavior of a revectorable function or overrideable method in an object framework.

Null_function @wikipedia


A
Anon

If the code is something like:

command = get_something_to_do()

if command:  # if not Null
    command.execute()

If you have a dummy object whose execute() method does nothing, and you return that instead of Null in the appropriate cases, you don't have to check for the Null case and can instead just do:

get_something_to_do().execute()

So, here the issue is not between checking for NULL vs. an exception, but is instead between the caller having to handle special non-cases differently (in whatever way) or not.


A
Aman Gupta

For my use case I needed to return a Map from method and then looking for a specific key. But if I return an empty Map, then it will lead to NullPointerException and then it wont be much different returning null instead of an empty Map. But from Java8 onward we could use Optional. The above is the very reason Optional concept was introduced.


R
Rob Wells

G'day,

Returning NULL when you are unable to create a new object is standard practise for many APIs.

Why the hell it's bad design I have no idea.

Edit: This is true of languages where you don't have exceptions such as C where it has been the convention for many years.

HTH

'Avahappy,


If you are unable to create an object, you should really throw an Exception. If you are unable to find an object that matches some query, then returning null is the way to go.
@Thilo, show me how to do that in C and I'll be most interested.
@RobWells C is not an object-oriented language, but this question is tagged as "oop"
@yegor256 You are right. And I missed the original OOP tag. But, as BS said a class in C++ is just a struct with some additional member functions added and some fancy memory management happening internally. But if an API is suuposed to return a struct, then returning NUL when you can't create the struct is often the convention.