views:

770

answers:

7

We recently had a code review . One of my classes was used so that I could return/pass more than one type of data from/to methods . The only methods that the class had were getters/setters . One of the team's members ( whose opinion I respect ) said that having a class like that is bad practice ( and not very OOP ) . Why is that ?

A: 

In general, you'll want to isolate the knowledge needed to operate upon a class into the class itself. If you have a class like this, either it is used in multiple places, and thus can take on some of the functionality in both of those places, or it is in a single place, and should be an inner class. If it is used in multiple ways, but in completely different ways, such that there is no shared functionality, having it be a single class is misleading, indicating a shared functionality where there is none.

However, there are often specific reasons for where these general rules may or may not apply, so it depends on what your class was supposed to represent.

John the Statistician
+11  A: 

There's an argument that classes should either be "data structures" (i.e., focus on storing data with no functionality) or "functionality oriented" (i.e., focus on performing certain actions while storing minimal state). If you follow that argument (which makes sense but isn't always easy to do) then there is nothing necessarily wrong with that.

In fact, one would argue that beans and entity beans are essentially that - data containers with getters and setters.

I have seen certain sources (e.g., the book "clean code") arguing that one should avoid methods with multiple parameters and instead pass them as a single object with getters and setters. This is also closer to the "smalltalk model" of named parameters where order does not matter.

So I think that when used appropriately, your design makes sense.

Uri
While I don't necessarily disagree with the "state" vs "functionality" class divide, I do feel the need to point out that this is taking the OO out of the OO.
Zarkonnen
I think the general idea behind this separation is to avoid overloading an object with noncore functionality and instead think of those sort of "functors" as classes as well.
Uri
+2  A: 

Check out this question.

Vincent Van Den Berghe
A: 

I think he might be confusing "not very OOP" for bad practice. I think he expected you to provide several methods that would each return 1 value that was needed (as you will have to use them in your new class anyway that isn't too bad).

Note that in this case you probably shouldn't use getters/setters, just make the data public. No this is "not very OOP" but is the right way to do it.

he_the_great
A: 

Maybe Josh Bloch offers some insight into this here.

jJack
+4  A: 

Note that there are two separate issues here.

  1. Is a "struct-like" class sensible?

  2. Is creating a class to return multiple values from a method sensible?

Struct-like classes

An object class should -- for the most part -- represent a class of real-world objects. A passive, struct-like java bean (all getters and setters) may represent a real-world thing.

However, most real-world things have rules, constraints, behaviors, and basic verbs in which they engage. A struct-like class is rarely a good match for a real-world thing, it's usually some technical thing. That makes it less than ideal OO design.

Multiple returns from a method

While Python has this, Java doesn't. Multiple return values isn't an OO question, per se. It's a question of working through the language limitations.

Multiple return values may mean that an object has changed state. Perhaps one method changes the state and some group of getters return the values stemming from this state change.

S.Lott
Python doesn't really allow for multiple return values, it wraps it up into a single type, tuple, and allows tuples/lists to be assigned to multiple variables in one statement.
he_the_great
@he_the_great: while true, it's a distinction so subtle as to be of almost no value for this situation. The issue here is one of conflating OO design and Java limitations. Python is OO but takes a different approach to some things.
S.Lott
+1  A: 

To be honest, it sounds fine to me. What alternative did the reviewer suggest?

Following OOP "best practices" and all is fine, but you've got to be pragmatic and actually get the job done.

Using Value Objects like this (OO speak for 'struct') is a perfectly legitimate approach in some cases.

madlep