3

If I have an ItemContainer class that contains, for example, items in an order, where each item is an Item object; is it best to have a method like:

ItemContainer->getItems() 

that returns an array containing Item objects, or is it better practice to do something like:

ItemContainer->getItem($itemNo) 

which returns a single item object for that item number, and forgoes the array. I realise this may be a trivial question or simply one of preference, but I'd like my app to adopt best practices from the start and I'm unsure which way to proceed. I'm writing in PHP, but I figured this pretty much applies to any OOP language.

  • 2
    Why not implement both? – user40980 Sep 18 '13 at 15:01
4

The principle reasons for not following the ItemContainer->getItems() approach and returning an array are:

  1. The initial implementation is likely to just return a reference to the original array, which would then allow the caller to modify the underlying collection. This is considered bad practice.
  2. The next work around is then to make a copy of the array and return that instead. This is bad because a caller may think they are still modifying the underlying collection, and it is also expensive for large collections.

If the caller really wants to have a copy of the array, then itemContainer->cloneItems() is much clearer, and less likely to be used inappropriately.

If the caller is just wanting to have a single item, then providing a

itemContainer->getItem(index && key)

is clear and efficient

If the caller is wanting to iterate over the items, then providing a

itemContainer->getItemIterator()

is clearer. Depending on the language you may implement

itemContainer->VisitItems(visitor)

where visitor is a visitor class, delegate or function pointer.

Hopefully, I've given you some ideas on how this can be approached differently

  • Thanks for the input. Returning an iterator does make sense to me, and it was extremely easy to implement, so I've gone with that approach. – r.bilgil Sep 18 '13 at 15:45
  • PHP arrays are copy-on-write. You cannot return an array by reference unless you explicitly do it with &, which is terribly unidiomatic. Using iterators unnecessarily is also an issue, because now you can't directly call array functions on the result. – ryeguy Sep 18 '13 at 16:02
  • +1 for the iterator. It abstracts away the underlying implementation so it can be changed without changing the clients of this class. – Marjan Venema Sep 18 '13 at 16:36
  • @ryeguy, If you want to manipulate the array, you should be considering writing that code inside the itemContainer class. – Michael Shaw Sep 18 '13 at 16:44
1

Why not both? Generally, since before 5.4 PHP did not support dereferencing the return of a function, it was commonplace to return an array of objects, and then if the key to the returned array was passed into that function, the function would return only that value.

It's PHP, so you are working in a dynamically typed language. If you want more surety, you could always create a separate object for storing the items, like ItemCollection. This would be an approach you would see in languages like Java or C#.

0

it depends:

  • returning an array allows you to pass it directly into the idiomatic foreach of the language

  • but using index-based get means you don't need to allocate the array and push all items into it.

  • Would you say there's anything that violates any OOP principles in returning an array of objects? I am concerned that any class that gets the item list will know that the items are stored as an array, for example. – r.bilgil Sep 18 '13 at 15:05
  • 1
    @r.bilgil don't return your internal reference to the array. Make a copy of it and return that. – user40980 Sep 18 '13 at 15:09
  • @MichaelT or a read only view – ratchet freak Sep 18 '13 at 15:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.