1

I couldn't think of a good way to word the title, sorry. But what I mean is, is it considered bad practice to do:

print get_array()[2]

over:

output=get_array()
print output[2]

(where get_array() is a function that returns an array, if that wasn't clear). Which method is better? Why?

  • possible duplicate of [Coding style for chained function calls](http://programmers.stackexchange.com/questions/210221/coding-style-for-chained-function-calls) – gnat Nov 07 '13 at 16:13
  • 2
    @gnat That page makes no reference to using arrays returned by functions. –  Nov 07 '13 at 16:17
  • 2
    That's perfectly acceptable idiomatic perl... and its a coding *style* and thus very much a matter of *opinion*. –  Nov 07 '13 at 16:20
  • 4
    It's prefectly idiomatic in anything where `[x]` is an operator that operates on arrays. It's not really much different than `print function() + 3`. Safety might be another issue, but if the function guarantees you'll get what you expect, there's no reason to store it separately. – Blrfl Nov 07 '13 at 16:21
  • 1
    in the context of your question, the difference is not substantial: it's about whether you store the intermediate result in a variable or go by chain – gnat Nov 07 '13 at 16:54

4 Answers4

5

Let me take a crack at this,

Think of the risks you're posing by accessing an array straight away.

  • Do you know for SURE, it's got data at the aforementioned index?
  • Is that data in the format you want?
  • If there was an Exception, like a null at that index, would your program be able to recover?

I cannot tell off the bat what language you're using... but, in any language it's a good idea to just double check the input of your functions before using them. This can save massive headaches when a codebase grows.

Erik
  • 361
  • 1
  • 7
  • +1. There's nothing wrong with the OP's first example, if they've got ironclad guarantees, and in many cases it may be the way to go, but.... The second one will make debugging easier if something goes wrong and makes adding in the checks easier when they prove to be needed. – RalphChapin Nov 07 '13 at 16:40
  • I just pointed out some real risks with their code. You are calling a function that **could** be iron clad.. but what code base really **is** ironclad? Why take the chance when it's just a simple check in the program..? Not worth it, and frankly it's poor decision to dismiss their risks their code is taking on when they are unsure of themselves. – Erik Nov 07 '13 at 21:58
  • I'm not disagreeing with with you. I don't _really_ believe in things like "ironclad guarantees", but sometimes it's fun to say "what if". – RalphChapin Nov 07 '13 at 22:25
  • @Erik: Any codebase where design by contract is in effect will be ironclad enough that if a contract makes a promise, it will be kept or an exception will be thrown before control returns to the caller. The idea behind DbC is to put the defensiveness as close to the code in question as possible instead of requiring that every caller duplicate that effort. – Blrfl Nov 09 '13 at 17:09
2

This entirely depends upon what you are planning on doing with the array and what you should/can do if the element you expect to be at a certain position isn't there.

Frequently, when this is done, it is returning multiple values (such as a first, middle and last names) which are actually independent -- the array is an ad hoc structure. The problem with this is that it isn't defined anywhere, and so is fragile. If you add enums to make it less fragile, you've spent about as much time, for not much gain - better to just define a structure and be done with it.

That said, sometimes you have to consume a API where this has been done, in which case, directly accessing the value is fine if the contract says it will always be there.

If you don't do anything with the returned value except access it once, putting it in a temp variable doesn't gain you anything (except possibly during debugging, where knowing what result is passed to a function can be useful).

jmoreno
  • 10,640
  • 1
  • 31
  • 48
2

From a design point of view, there seems to be a problem here - you're accessing a function that returns a set of values, take one specific value from it, and discard the rest.

Doesn't this feel like a waste, retrieving values you have no use for? Wouldn't it make more sense to replace get_array()[2] with a specific call, get_item(2)?

Avner Shahar-Kashtan
  • 9,166
  • 3
  • 29
  • 37
-2

It is always good practice to separate your code's IO from its internal computations. The coding-related StackExchange sites are littered with code that mixes Print (or Println or putStr or whatever is the local dialect) inside complex functions. This is bad practice.

In the case of output (as in your example), mixing output statements with computation tends to make the final output of the program dependent on the internal structure of the code. It then becomes difficult to alter that code without breaking the output. For example, you might want to print out a sorted sequence of lines; you happen to extract them from some structure which automatically sorts them, so you naively add print statements as you compute each line. This works. Then you switch to storing the intermediate calculations in a structure which doesn't sort them. Broken output which requires you to remove the print statements, add a sorting step and then put the print statements after that.

If you separate computation from presentation, you don't have this problem.

In your specific example, I don't think either choice is good, because both might fail. Even if you get a valid array, what if it doesn't have a second row? This is a problem you should resolve before your output step. Your second example is actually worse, because it doesn't resolve the problem so adds a pointless extra step.

output=get_array()[2]
print output

Would be better. The retrieval of the data and its output can then be cleanly separated.

itsbruce
  • 3,195
  • 17
  • 22
  • 2
    This is a complete strawman. Splitting the code into two statements (regardless of *where* you split) does not magically make it safe, or perform separation of concerns. I cannot see how separation of concerns (or for that matter, the Law of Demeter) might be substantially violated in this questions code. – amon Nov 07 '13 at 17:49
  • @amon No, it doesn't magically make it safe, but it makes it easier to make it safe. Separating print statements from computation is good practice. Splitting them arbitrarily, so that some of the computation is left bundled with the output, is *worse* than useless, so that his second example is more than a waste of time. Since he wanted a comparison of the two, this is relevant. – itsbruce Nov 07 '13 at 17:55