views:

3722

answers:

37

Do you subscribe to this school of thought?

Assertion:

"I always return an Enum, Array, or an Iterator, never a Boolean, NULL, or an Instance."

Reasoning:

Code using Enums instead of Booleans. Return Empty Arrays instead of nulls, and return an Iterator even if the calling Method will only take the first value; instead of, returning an Instance. All these measures are intended to improve the extensibility of the codebase with negligible drawbacks. And furthermore, because of these points, it should be adopted as a best-practice.

+2  A: 

Uh, what's the context? Blanket statements like that are dangerous and usually result from misunderstanding something.

Jonathan Allen
+10  A: 

In C# at least, returning an Array is probably not a good idea. For reasons why, see this blog post by Eric Lippert.

In a nutshell, if you are returning an array that is a private member of your class, then you will be exposing your class's internal state to the caller, and they can then make changes which may have unintended side effects.

To get round this, you would have to create a copy of the array every time you return it, which is an O(n) operation that may have a negative impact on performance for large arrays.

All in all, his statement seems a bit muddled to me. There's no reason why you shouldn't return bool or an instance (returning an instance of something is what the Abstract Factory design pattern and friends are all about) and while null can be a pain in the neck at times, there may be some places where you will want to return it, e.g. to indicate a record not found in a database.

jammycakes
Same in Java, where even for 'final' arrays, only the reference to the array itself is immutable, the elements can still be modified.
sk
Good point about abstract factories.
fluffels
+147  A: 

It's from the school of thought called inexperience.

"Always" and "Never" are words that should "Never" be used in programming :p

In every case, you do what makes sense. Even returning null has a place.

Gerald
you can usually tell the level of experience of a typical systems analyst by how much he hedges anything he says...allegedly ;-)
Steven A. Lowe
So what you are saying is: All specification and copy writers are inexperienced?
leppie
Yep, if they're saying that for general programming practices, rather than the scope of their specifications, they saying so either makes them inexperienced or incompetent.
Gerald
Incidentally I've rarely seen a specification that would say anything like that without a caveat. And any specification writer that does so without some degree of flexibility is almost certainly inexperienced.
Gerald
@Gerald: Indeed. For an example, look at http://www.w3.org/Protocols/rfc2616/rfc2616.html (the HTTP/1.1 spec): the "SHOULD" is used much more often than "MUST"
Piskvor
+1 for talking sense
MatthieuF
And of course, the 'never' above applies to itself. But it all comes down to how surprising the guidance is. Always and never can apply only to gudiance which nobody with even minimum experience would raise an eyebrow at.
Pontus Gagge
Sounds like Perl - There's more than one way to do it (TMTOWTDI) http://en.wikipedia.org/wiki/There_is_more_than_one_way_to_do_it
gbanfill
A: 

Yuck!!

Re enums -> no standards Re Arrays -> unless your arrays have size, it won't work, if they do, that's overhead. Re Iterator -> more cost at (often) no benefit.

BCS
+4  A: 

As best-practices, I'd have to say this is fairly poor. Sometimes you do need to return a boolean, because the function checks for the existence of some sort of condition. At times it's good to think if you can return something 'more' than just true or false - for instance, when searching of string A is in string B, most languages have a function that returns either 'No' or 'Yes and it's at position X', by returning that position. But there are times when what you need is either yes or no, and restricting that to the point where someone has to write an Enum with the values of 'True' and 'False' is ridiculous.

John Fiala
+22  A: 

The best interpretation I could come up for that statement is "prefer enumerations to booleans, empty arrays to nulls, and an iterator to a container rather than the container itself".

If it's not clear from code what the boolean represents then an enum can make things clearer. If a method returns an array, then yes it should return an empty array as opposed to a null if there are no elements to return. As for returning an iterator, that's mainly a C++ idiom, although in C# returning an IEnumerable<T> or IList<T> is better than returning the underlying implementation.

Mark Cidade
+52  A: 

enum vs bool

Using an enumeration instead of a boolean is a practice where you use a simple two-member enum instead of a bool. For instance:

window->display(false);      // False? False what?
window->display(WINDOWED);   // Same as "fullscreen = false"
window->display(FULLSCREEN); // Same as "fullscreen = true"

if (!create_instance())          // "If not create instance?" Wha?
if (create_instance() == FAILED) // Much more explicit and clear.
                                 // Also possibly a required check if an
                                 // implicit cast to bool does not exist.

Granted it's a bit of a style thing, but the enhanced readability and, to some degree, type safety makes it worth considering.


array vs null

I'm guessing this is a language-specific case for a Null Object -- C# doesn't like foreach being called on null, but will work properly with an array with no elements (or so I'm told -- I'm a C++ guy myself). It isn't always applicable, but often you can create an object that effectively behaves as a "null" object, avoiding the risk of the program blowing up if the user doesn't check for this case himself. For example:

/// Returns foo's name, normalizing it if it isn't already.
String* get_name() {
    String* name = foo.get_name();
    if (name == nullptr)
       return &EMPTY_STRING;
    name->normalize();
    return name;
}

Can be replaced with:

String* get_name() {
    String* string = foo.get_name();
    string->normalize();
    return string;
}

Admittedly not the best example, but hopefully it communicates the idea behind this.


iterator vs instance

I'm guessing this is saying: If you have a container and an element is requested, return an iterator to that position rather than just the single element. Presumably the iterator can be dereferenced easily enough if only that one element was desired, and if the position is needed as well, it's already right there. (Note that I personally don't agree with this one.)

Tyler Millican
array vs null. In C# you can do a foreach (object item in obejctlist). If objectlist is null the code explodes. An empty list will just skip the foreach.
Dead account
@Ian Quigley: exactly. I generally (note the use of the word generally) return an empty list rather than a null value because it makes the calling code much simpler
MatthieuF
enum vs bool: that example only works if the methods that return the enum are used only by the class that defines it. your code would be a lot more cryptic if you try to call the method from elsewhere. Like MyClass.createInstance() == MyClass.CreationResults.FAILED
Pablo Fernandez
+3  A: 

Lets take it in parts. When a method is being used to detect a state or condition, a simple boolean seems inadequate. A enumeration on the other hand can represent a domain in a better fashion. Think of detecting a user's choice, say a preferred colour or marital status. A simple boolean would be insufficient, on the other hand a enumeration can actually reflect the modelled choices more accurately without compromising type safety.

The second part is generally a programming style used when a method needs to return multiple values. I can think of two instances where this becomes more agreeable.

  1. Modular code needs to have a single point of return, this makes code more 'refactorable'. So an empty list or array is declared at the beginning of the method and populated as part of the logic. Returning a null would essentially show that there are two exit points.
  2. The code that uses methods that may return null, is forced to check for nulls. On the other hand, If we return an empty array or a iterator, the looping construct would automatically eliminate the need for null check. The empty array/iterator imply no run. Thus reducing the extra lines of code.
questzen
+3  A: 

I'm not about to create an enum just so I can have a "proper" return value for my equals method. An empty array should be preferred over returning null, just as an Iterator should be over an instance. As others have said before me, you should never speak in absolutes.

Bill the Lizard
+5  A: 

Using enums instead of Booleans brings to mind a classic:

http://thedailywtf.com/Articles/What_Is_Truth_0x3f_.aspx

enum Bool 
{ 
    True, 
    False, 
    FileNotFound 
};

Joking aside, I agree with the reasoning brought forth by Tyler Millican:

Using an enumeration instead of a boolean is a practice where you use a simple two-member enum instead of a bool. For instance:

window->display(false);      // False? False what?
window->display(WINDOWED);   // Same as "fullscreen = false"
window->display(FULLSCREEN); // Same as "fullscreen = true"

However, a properly commented interface and an IDE that can display live documentation on mouse hovers can make things equally clear:

/**
 * Displays the window
 * @param bFullscreen Set to true to display the window
 *                    in fullscreen mode, false otherwise.
 */
 void display(bool bFullscreen);
Ates Goral
I was going to post the same thing!
Valerion
Boo for `bFullscreen` ! Boo to systems Hungarian!
Chris Lutz
+29  A: 

Extensibility is nice. But sometimes you just really don't need it. Take a function like this, for example:

bool IsWindowVisible() {...}

Boolean is absolutely appropriate in this case. I could use an enum for this, but why? What would the answers be? True, False, and Maybe? Extensibility is not an issue here. I can think of no situation in which I would extend a function called "IsWindowVisible" in such a way that a simple true/false would not be appropriate. If I want the function to do more than tell me whether a window is visible, then I should create a new function for that and deprecate this one (or keep both).

People like your graduate are usually the kind of people who write loose, inefficient code. A piece of code I was working on at work in the past year returned an ArrayList of items to display on a webpage in case of error. Leaving aside the fact that it could have been a generic List or a StringCollection instead (both of which would have been more efficient in this particular case), the programmer's excuse was that you should "never return a null." So.... rather than return a null, he stuck to his principles, and now when a button click ripples through the inheritance chain of this particular module of code, a hundred empty array lists are created when there are no errors, rather than the whole thing simply returning a great big nothing.

My point: "Never" take advice from people who tell you never or always to do things. "Always" write your code to be as efficient and effective as possible.

DannySmurf
true, false, and FILE_NOT_FOUND naturally!
mike nvck
read last line, decide "ok, wont take advice from you either". :P. +1 anyway.
Kent Fredric
+16  A: 

Concerning returning empty arrays instead of null in Java, see Item 43 of Joshua Bloch's Effective Java Second Edition (Return empty arrays or collections, not null).

The reason is that returning null forces your callers to treat null as a special case.

E.g. suppose getSomePeople() can return null:

final Person[] persons = getSomePeople()
//Don't forget to test for null, or you can get a NullPointerException!
if(null != persons) {
    for(final Person person : persons) {
        // ...
    }
}

Same example if getSomePeople() returns new Person[0]:

for(final Person person : getSomePeople()) {
    // ...
}

Safer, and easier to read.

And if profiling reveals that creating empty arrays costs too much, store your empty array in a final static member (empty arrays are immutable in Java, and can be shared freely).

There is also

  • Collections.emptySet(),
  • Collections.emptyList(),
  • Collections.emptyMap(),

three generic methods that return immutable empty collections.

As for always returning an Enum instead of a boolean, or an iterator instead of an instance, it looks like a very bizarre thing to do, but maybe it's just me...

if (YES == isInflatable()) // I'd hate to read this!

Cheers!

Sébastien RoccaSerra
+3  A: 

Umm, why? I'd like to hear a justification for this rule.

Sure there are cases where returning an enum instead of a boolean, for example, would improve clarity. If I have a function called, say, getEmployeeType(), returning an enum that can be SALESMAN or ENGINEER would make sense. Returning a boolean would be crypticm, and not allow for obvious extensions to other types of employee. But suppose instead I had a function isEmployeeSalaried(). Now a true or false would be very clear. Sure, I could have an enum with SALARIED and HOURLY, but that just adds the extra hassle that anyone using the function has to check the list of possible values, whereas with a boolean he immediately knows that the only possible values are true and false.

The idea of returning an array or enumeration even when the caller will only ever use the first value strikes me as insane. Anyone looking at the call is going to be led to the false assumption that this function could return more than one. It sounds like you're deliberately trying to trick the user of your API. To describe this as helping extensibility ... You mean that now the function only returns one element but someday you may modify it to return more than one? Really now. If I have a function to, say, return the logarithm of the passed-in value, is it really plausible to say that someday a number might have two logarithms? Or if it gets the customer's balance, that in the future customer's may have many current balances? If the definitions changed so radically that that somehow made sense, surely we would have to re-examine what any caller was doing with the return value anyway.

By that reasoning, my not just always return an Object (java) or void (C++), so no matter what change you ever make, it will still be valid? The obvious problem with making your return values any more vague than absolutely necessary is that it forces the human user of your API to do extra research to figure out what you might really return, and then to write extra code to extract the specific value from the generic. And I presume if the user knows that you only ever populate the first value in an array or that your enum is always TRUE or FALSe, he's going to just read the first value from the array, or just check the return value for equals TRUE, so any extra flexibility is in your imagination. Or do you suppose that every caller of "Object[] log(float x)" is actually going to write code to loop through the array and check the type of each value to see if it's a String or a file reference, and then do something meaningful with every imaginable type?

Mark
+3  A: 

All of these techniques (if applied like the OP suggested) make the interfaces to classes/libraries/whatever more clumsy.

If you need this level of "extensibility" for all of your return types I would suggest that maybe you have not done the appropriate level of analysis to really nail down what the interfaces should be.

+2  A: 

Using an enumeration instead of a boolean is a practice where you use a simple two-member enum instead of a bool.

Unlike a boolean, an enumeration is extensible. First you are told that a gate can either be open or shut and later they'll want to know whether it's locked :)

onedaywhen
Open + Locked might also be a possibility; not an intelligent one, but most locks can be locked when open. Bitmask, struct, or two separate variables, please.Perhaps a better example is OPEN, SHUT, OPENING, SHUTTING.
Mikeage
My French windows have cabin hooks that should be engaged to 'lock them open' to ensure they don't slam in the wind. My lounge door has a wedge so that it can be 'locked half open' when the fire is lit so we don't asphyxiate. Doors are sooo complex :)
onedaywhen
See the "YAGNI" comment above. Changes come, but never as you anticipate. You;ll have to refactor either way, Better to refactor something simple.
Anthony
+2  A: 

Absolute extensibility for the sake of extensibility is a nonsense. Readability and performance is often more important. At least I need to understand how the code works before I have a chance to extend it.

Using enums where boolean is enough decreases readability. Returning empty arrays instead of null sometimes decreases performance (this should be carefully profiled).

Anyway this "always" and "never" are just another set of limitations which can hurt badly if applied absolutely every here and there.

sharptooth
+6  A: 

It sounds like pieces of good advice mixed up and misapplied.

Good advice is passing enums instead of booleans.

SomeFunc (someObject, true, true, true, false);

vs

SomeFunc (someObject, IS_ROUND, IS_BLACK, IS_SHINY, IS_NOT_ALIVE);

It's much more clear what the second function does. However there's a good argument to be made that good documentation combined with Intellisense can alleviate this concern, and perhaps it could be argued that adding all these specific enums could cause clutter. I think of it as a good general practice to name your parameters in this way, but perhaps if you have a series of bools like that it indicates another problem in the function design.

I think the grad student in question may have misapplied this advice to return values. In the case of a return value there is only one and it tends to have an unambiguous meaning if it's bool (success or failure). For return values I wouldn't find as much value in the practice of using named parameters rather than the (often) built-in boolean type.

Regarding returning arrays, I +1'd Sébastien Rocca-Serra's answer because it's a great point for Java specifically. In C++ a much better "best practice" would be to never return an array ever. The point Sebastien raised is taken, though... returning a valid value in every case (even if that value is a stub or sentinel) can lead to easier error handling in the calling code. In this case I think the spirit of the rule is much better than the letter of the rule as relayed to us.

As for returning an iterator, I don't have much comment. Given the trends so far I assume it's a skewed version of some practical advice, so if others can enlighten me on the pearl of wisdom in it I'd be appreciative.

Dan Olson
I've become far to dependent on intellisense. When I use a text application or IDE that doesn't support something of it's ilk I feel lost... But I suppose before intellisense, the vast majority of my coding time was spent knee deep in documentation. Now I get to see bits of documentation that actually makes sense. :P
Sekhat
this is a case where i'm glad my main language has named arguments. This allows you to use obvious names with appropriate types, and don't even worry about getting the order right. I have, on rare occasion, bolted this into a language that doesn't support it natively, by passing an associative container instead of a list of arguments. Hmm... there's a lot of arguments to that function, are you sure you didn't miss any? (http://en.wikiquote.org/wiki/Alan_Perlis)
TokenMacGuy
+1  A: 

As a C and assembly developer, I'll leave the higher level abstractions for others. However, what prototype would he propose for malloc? What exactly should it return, if it can't allocate memory?

I suppose this same comment applies to just above all functions that take or return pointers, but this is a particularly glaring hole in his logic.

As has been mentioned above: always make sure never to say never!

Mikeage
+2  A: 

Code using Enums instead of Booleans. Return Empty Arrays instead of nulls, and return an Iterator even if the calling Method will only take the first value; instead of, returning an Instance

There's been a lot on enums and Booleans already. Some of the other topics. I agree with returning Empty Arrays instead of nulls, because it helps prevent NullReferenceExceptions or the like if the developer using the assembly forgets to add in Null checks. For instance

foos[] = app.GetFoos();
for (int i = 0; i < foos.Length; i++)
{

}

That won't error if the result of app.GetFoos() is empty and helps prevent potential run time errors.

Return an iterator over an instance? I've never heard that one before, but really I think it needs to be applied correctly. For example... ToString()... That should never return more than 1 string, so why return an iterator? I think some common sense and forward thinking should be applied to that prinicple.

Ian
+23  A: 

YAGNI - You ain't gonna need it

It sounds like a typical student thing: their project is the world's greatest software project ever known to mankind, it will surley live forever and it must therefore be prepared to adapt to all possible changes.

Changes will come, but sadly, seldom in the ways you spent so much preparing for. :-(

danbystrom
+1 - This is the most salient observation and one I would have entered if you hadn't. In the course of writing hundreds and hundreds of methods, this would add an enormous amount of work. For what? To save a bit of work that *might* be needed in the future for a specific method or two?
Mark Brittingham
Mod parent up. This is so true, in my experience.
Anthony
"Changes will come, but sadly, seldom in the ways you spent so much preparing for. :-(" VERY VERY true. +1
Eric Palakovich Carr
+4  A: 

Always return a null, never a "null"

I have seen methods returning a String with value "null" !

Guido
+2  A: 

I strongly object to the enum VS bool idea. following these kind of rules blindly can lead to some hillarious WTFs:

enum EReturnVal {
   True,
   False,
   Maybe
}

EReturnVal isPointInsidePolygon(); // WTF??
EReturnVal isReactorCritical(); // WTF??

some questions must have and will always have a true/false answer. Answering anything else doesn't make sense.

shoosh
+1  A: 

Re: null.

The Null Object pattern is certainly a good practise, and an often underutilised one. That doesn't mean that you should use it always of course.

Re: iterator vs. instance

I'd say that depends on the semantics of the function. Two is an impossible number, but 1 is quite fine with me.

Re: enum vs. boolean

There are cases where this may be a applicable, and then - by all means - use an enum, but booleans have their place as well.

troelskn
A: 

Simple answer is NO, Further flaming, if you think extensibility in that level 98% of the time you can't code anything in real world.

dr. evil
A: 

My first thought was :

"This way of thinking tends to break semantic and readability to enforce extensibility."

But thinking honestly about it, if it's an habit and does not affect performances in your project, maybe it has its good paybacks.

I'd say to try it before jugging it, but it would bore me to death to type [0] after a isSomething() method ;-) Anyway, it's frankly a psychological issue.

Eventually, this debate is for hight level programmer only. In C, you wouldn't even ask, would you ?

e-satis
+10  A: 

All I can offer is my simple opinion, a predicate had better return a boolean

if is_prime(foo):
   etc...

is quite nice:

if prime_or_composite(foo) == NUMBERS.PRIME:
    etc...

Is heinous. Predicates happen all over the place. Love them!

TokenMacGuy
+1  A: 

Returning empty array/lists rather than nulls makes sense, it allows the calling code to be simpler – no special-case code to deal with the nulls.

I'm not sure what you mean by "never return an instance" – are you saying always a Array/List/Iterator rather than a single object? That would be an odd and pointless practice.

The other interpretation, returning an interface over a concrete type (e.g. returning IList<Foo> or IEnumerable<Foo> over List<Foo>) is a good practice, but I wouldn't call it a "must" and "always" practice.

As for not using booleans, this wrong idea is debated at Are booleans as method arguments unacceptable?

Anthony
+1  A: 

As a C/C++ programmer, the only way I would fill in those blanks that Always applies is:

"Always return an immediate value, or a pointer to heap or static memory, never a pointer to the stack."

AShelly
+1  A: 

Personally, and this is way too far down the list right now to matter, I never use the term "Best Practices". To state that something is the best is to imply that questioning it is out of bounds. I'd place that right up there with "always" and "never" as terms to avoid.

As for the particulars of your graduate student's assertion, I would chalk that up to being a graduate student rather than a practicing professional.

KevDog
+1  A: 

Their Reasoning: Code using Enums instead of Booleans. Return Empty Arrays...

You call "Code using Enums instead of Booleans" a reasoning?

To answer your last question, of course, no. If I did, I wouldn't be able to answer in the first place, because your question requires a boolean answer. So does my code: it returns a boolean when it is required.

Daniel Daranas
+1  A: 

There is another reason to consider this approach: On some compilers, returning anything other than register-sized objects adds extra operations to truncate, sign extend and/or mask bits. This also applies to parameters to functions and methods.

Whether this extra overhead is significant, and worth the reduction in readability, I leave as an exercise for the reader.

dominic hamon
+1  A: 

Return an Enum, Array, or an Iterator when you need to return an Enum, Array, or an Iterator. Return a Boolean, NULL, or an Instance when you need to return a Boolean, NULL, or an Instance.

If the answer is not 6 weeks, it's normally "it depends".

Only Sith deal in absolutes, right?

Aaron Daniels
+2  A: 

Code using Enums instead of Booleans. Return Empty Arrays instead of nulls, and return an Iterator even if the calling Method will only take the first value; instead of, returning an Instance

I worked on an ORM class for PHP, called Zend_Db_Table. This class had a method find() which returns a database row, corresponding to the primary key value you give as an argument. Sounds like a case that can return only zero or one rows, right? So either one object instance, or else null.

However, it also supported an argument that was an array of primary key values. So it had the potential for returning multiple rows, one row, or zero rows. I changed this find() function to return a Rowset object, which we were using as a collection object for rows. That way it simplified usage in calling code: you always knew the return type was an Iterable. You didn't have to write a case statement to test the type of the return value.

So there are certainly cases where your grad student's guidelines make sense.

  • Enum is extensible and carries more information than a boolean. A function that returns one of two distinct values, but may offer more in the future, is a clear case where a boolean is not the right choice.

  • If the non-null result of a function is an array, then returning a null means the calling code has to handle this special case, or else coerce the null to an empty array anyway, before it can iterate over the result.

  • If the function may return an array of variable length, then don't "optimize" the singleton case by returning an instance. Return an array containing one element. Likewise, if the array has zero elements, return an array with zero elements.

But these guidelines are not universal, and it's naive to try to make them rules to apply in all situations. There are just as valid cases where bool, null, and a single object instance is appropriate:

  • It would be nonsensical for a function that is a predicate to return values other than true and false. So there's no reason to use an enum in those cases.

  • A function that normally returns a single instance and has no reason to return an array may return null to signify "no value."

  • A function that normally returns a single instance has no reason to return an iterator.

Bill Karwin
+1  A: 

Extensibility and flexibility are good objectives -- but the experienced developer also recognizes that, paradoxically, the more flexibility you try to design in, the more complex the system becomes, and therefore hard to evolve and maintain since even simple changes can have unobvious implications. The ultimate in flexibility is the ultimate in brittleness.

Complexity costs.

It's the same with Dependency Injection, really. It's a good thing, when you have a reasonably accurate guess at what points changes are likely. If you try to inject configurability everywhere, you build a monster not even its creator will be able to understand.

Pontus Gagge
A: 

Maybe your graduate student is trying to hide his lack of understanding behind ridiculous dogma?

+2  A: 

In my experience returning a bool is fine since many methods have a boolean like quality to them. However, passing bools typically causes confusion as the user has to examine the prototype to infer what the bool means.

It's better to create an enumeration and pass this, even if it does mean a bit more typing but it's much more self-documenting and this is always better in the long run.

Sean
+1  A: 

My first thought: Wow, this is a lot of comments for anything said by a grad student. You should have stopped listening when you realized he wasn't describing how to make a bong out of some common household good. Second thought: So he wants me to make my code needlessly complex and hard to read because I might want to extend a few functions some unknown time in the future? Yeah, that sounds good.
I think it is clear this guy has not worked on a large project with a team before. Go ahead and try to program that way on some teams I have been on. I think they would have burned him at the stake after the first code review.

ScottD