47

In legacy code I occasionally see classes that are nothing but wrappers for data. something like:

class Bottle {
   int height;
   int diameter;
   Cap capType;

   getters/setters, maybe a constructor
}

My understanding of OO is that classes are structures for data and the methods of operating on that data. This seems to preclude objects of this type. To me they are nothing more than structs and kind of defeat the purpose of OO. I don't think it's necessarily evil, though it may be a code smell.

Is there a case where such objects would be necessary? If this is used often, does it make the design suspect?

12
  • 1
    This doesn't quite answer your question but seems relevant nonetheless: stackoverflow.com/questions/36701/struct-like-objects-in-java
    – Adam Lear
    Dec 27, 2010 at 14:39
  • 3
    I think the term you are looking for is POD (Plain Old Data).
    – Gaurav
    Dec 27, 2010 at 14:51
  • 10
    This is a typical example of structured programming. Not necessarily bad, just not object oriented. Dec 27, 2010 at 15:02
  • 1
    shouldn't this be on stack overflow?
    – Muad'Dib
    Dec 27, 2010 at 15:42
  • 8
    @Muad'Dib: technically, it is about programmers. Your compiler doesn't care if you use plain old data structures. Your CPU probably enjoys it (in the "I love the smell of data fresh from the cache" sort of way). It's people who get hung up on "does this make my methodology less pure?" questions.
    – Shog9
    Dec 27, 2010 at 18:42

10 Answers 10

75

Definitely not evil and not a code smell in my mind. Data containers are a valid OO citizen. Sometimes you want to encapsulate related information together. It's a lot better to have a method like

public void DoStuffWithBottle(Bottle b)
{
    // do something that doesn't modify Bottle, so the method doesn't belong
    // on that class
}

than

public void DoStuffWithBottle(int bottleHeight, int bottleDiameter, Cap capType)
{
}

Using a class also allows you to add an additional parameter to Bottle without modifying every caller of DoStuffWithBottle. And you can subclass Bottle and further increase the readability and organization of your code, if needed.

There are also plain data objects that can be returned as a result of a database query, for example. I believe the term for them in that case is "Data Transfer Object".

In some languages there are other considerations as well. For example, in C# classes and structs behave differently, since structs are a value type and classes are reference types.

15
  • 27
    Nah. DoStuffWith should be a method of the Bottle class in OOP (and should most probably be immutable, too). What you have written above is not a good pattern in an OO language (unless you are interfacing with a legacy API). It is, however, a valid design in a non-OO environment. Dec 27, 2010 at 16:00
  • 11
    @Javier: Then Java isn’t the best answer either. Java’s emphasis on OO is overwhelming, the language is basically no good at anything else. Dec 27, 2010 at 16:09
  • 11
    @JohnL: I was of course simplifying. But in general, objects in OO encapsulate state, not data. This is a fine but important distinction. The point of OOP is precisely not to have a lot of data sitting around. It is sending messages between states that create a new state. I fail to see how you can send messages either to or from a methodless object. (And this is the original definition of OOP so I wouldn’t consider it flawed.) Dec 27, 2010 at 16:53
  • 15
    @Konrad Rudolph: This is why I explicitly made the comment inside the method. I agree that methods that affect instances of Bottle should go in that class. But if another object needs to modify its state based on the information in Bottle, then I think my design would be fairly valid.
    – Adam Lear
    Dec 27, 2010 at 17:12
  • 14
    @Konrad, I disagree that doStuffWithBottle should go in the bottle class. Why should a bottle know how to do stuff with itself? doStuffWithBottle indicates that something else will do something with a bottle. If bottle had that in it, that would be tight coupling. However, if the Bottle class had a isFull() method, that would be totally appropriate.
    – Nemi
    Dec 27, 2010 at 18:31
30

Data classes are valid in some cases. DTO's are one good example mentioned by Anna Lear. In general though, you should regard them as the seed of a class whose methods haven't yet sprouted. And if you are running into a lot of them in old code, treat them as a strong code smell. They are often used by old C/C++ programmers who have never made the transision to OO programming and are a sign of procedural programming. Relying on getters and setters all the time (or worse yet, on direct access of non-private members) can get you into trouble before you know it. Consider an example of an external method that needs information from Bottle.

Here Bottle is a data class):

void selectShippingContainer(Bottle bottle) {
    if (bottle.getDiameter() > MAX_DIMENSION || bottle.getHeight() > MAX_DIMENSION ||
            bottle.getCapType() == Cap.FANCY_CAP ) {
        shippingContainer = WOODEN_CRATE;
    } else {
        shippingContainer = CARDBOARD_BOX;
    }
}

Here we've given Bottle some behavior):

void selectShippingContainer(Bottle bottle) {
    if (bottle.isBiggerThan(MAX_DIMENSION) || bottle.isFragile()) {
        shippingContainer = WOODEN_CRATE;
    } else {
        shippingContainer = CARDBOARD_BOX;
    }
}

The first method violates the Tell-Don't-Ask principle, and by keeping Bottle dumb we have let implicit knowledge about bottles, such as what makes one fragle (the Cap), slip into logic that is outside the Bottle class. You have to be on your toes to prevent this sort of 'leakage' when you habitually rely on getters.

The second method asks Bottle only for what it needs to do its job, and leaves Bottle to decide whether it is fragile, or larger than a given size. The result is a much looser coupling between the method and Bottle's implementation. A pleasant side-effect is that the method is cleaner and more expressive.

You'll rarely make use of this many fields of an object without writing some logic that ought to reside in the class with those fields. Figure out what that logic is and then move it to where it belongs.

4
  • 2
    Can't believe this answer has no votes (well, you have one now). This might be a simple example, but when OO is abused enough you get service classes that turn into nightmares containing tons of functionality that should have been encapsulated in classes.
    – Alb
    Feb 28, 2012 at 17:21
  • "by old C/C++ programmers who have never made the transision (sic) to OO" ? C++ programmers usually are pretty OO, as it's an OO language, i.e. the whole point of using C++ instead of C. Apr 15, 2018 at 21:06
  • 1
    @nappyfalcon Writing code in an OO language doesn't necessarily make it OO code; many C++ programmers sure try for OO, but it should be clear that they don't automatically succeed. Also note C++ is general-purpose paradigm, not OO exclusively.
    – geometrian
    Oct 22, 2022 at 12:50
  • @nappyfalcon C++ is also C and those old programmers actually made a transition from C to C++ by either transforming existing C code, at least to some degree, or starting new C++ projects with experience, habits, and a mindset rooting in C programming. A lot of beginner level C++ books started with the basics of C before they got to C++. There is nothing in the C++ language that forces you to write OO code. Even languages like Java just force you to put everything into classes, which doesn't automatically lead to OO code. On the other hand there is C code that is very much OO.
    – BlackJack
    Dec 5, 2022 at 14:14
9

If this is the sort of thing you need, that's fine, but please, please, please do it like

public class Bottle {
    public int height;
    public int diameter;
    public Cap capType;

    public Bottle(int height, int diameter, Cap capType) {
        this.height = height;
        this.diameter = diameter;
        this.capType = capType;
    }
}

instead of something like

public class Bottle {
    private int height;
    private int diameter;
    private Cap capType;

    public Bottle(int height, int diameter, Cap capType) {
        this.height = height;
        this.diameter = diameter;
        this.capType = capType;
    }

    public int getHeight() {
        return height;
    }

    public void setHeight(int height) {
        this.height = height;
    }

    public int getDiameter() {
        return diameter;
    }

    public void setDiameter(int diameter) {
        this.diameter = diameter;
    }

    public Cap getCapType() {
        return capType;
    }

    public void setCapType(Cap capType) {
        this.capType = capType;
    }
}

Please.

10
  • 15
    The only problem with this is that there is no validation. Any logic as to what a valid Bottle is should be in the Bottle class. However, using your proposed implementation, I can have a bottle with a negative height and diameter - there's no way to enforce any business rules on the object without validating every time the object is used. By using the second method, I can ensure that if I have a Bottle object, it was, is, and always will be a valid Bottle object according to my contract.
    – Thomas Owens
    Dec 27, 2010 at 20:35
  • 7
    That's one area where .NET has a slight edge over properties, since you can add a property accessor with validation logic, with the same syntax as if you were accessing a field. You can also define a property where classes can get the property value, but not set it.
    – JohnL
    Dec 27, 2010 at 22:04
  • 3
    @user9521 If you are sure that your code will not cause a fatal error with "bad" values, then go for your method. However, if you need further validation, or the ability to use lazy loading, or other checks when data is read or written, then using explicit getters and setters. Personally I tend to keep my variables private and use getters and setters for consistency. This way all my variables are treated the same, regardless of validation and/or other "advanced" techniques.
    – Jonathan
    Dec 28, 2010 at 4:29
  • 2
    The advantage of using the constructor is that it makes it much easier to make the class immutable. This is essential if you wish to write multi-threaded code. Dec 29, 2010 at 17:36
  • 8
    I would make the fields final whenever possible. IMHO I would have preferred fields to be final by default and have a keyword for mutable fields. e.g. var Dec 30, 2010 at 1:08
6

As @Anna said, definitely not evil. Sure you can put operations (methods) into classes, but only if you want to. You don't have to.

Permit me a small gripe about the idea that you have to put operations into classes, along with the idea that classes are abstractions. In practice, this encourages programmers to

  1. Make more classes than they need to (redundant data structure). When a data structure contains more components than minimally necessary, it is un-normalized, and therefore contains inconsistent states. In other words, when it is altered, it needs to be altered in more than one place in order to remain consistent. Failure to perform all coordinated changes makes it inconsistent, and is a bug.

  2. Resolve problem 1 by putting in notification methods, so that if part A is modified, it tries to propagate necessary changes to parts B and C. This is the primary reason why it is recommended to have get-and-set accessor methods. Since this is recommended practice, it appears to excuse problem 1, causing more of problem 1 and more of solution 2. This results not only in bugs due to incompletely implementing the notifications, but to a performance-sapping problem of runaway notifications. These are not infinite computations, merely very long ones.

These concepts are taught as good things, generally by teachers who haven't had to work within million-line monster apps riddled with these issues.

Here's what I try to do:

  1. Keep data as normalized as possible, so that when a change is made to the data, it is done at as few code points as possible, to minimize the likelihood of entering an inconsistent state.

  2. When data must be un-normalized, and redundancy is unavoidable, do not use notifications in an attempt to keep it consistent. Rather, tolerate temporary inconsistency. Resolve inconsistency with periodic sweeps through the data by a process that does only that. This centralizes the responsibility for maintaining consistency, while avoiding the performance and correctness problems that notifications are prone to. This results in code that is much smaller, error-free, and efficient.

4

Agree with the Anna Lear,

Definitely not evil and not a code smell in my mind. Data containers are a valid OO citizen. Sometimes you want to encapsulate related information together. It's a lot better to have a method like...

Sometimes people forget to read the 1999 Java Coding Conventions which make it very plain that this kind of programming is perfectly fine. In fact if you avoid it, then your code will smell! (too many getters/setters)

From Java Code Conventions 1999: One example of appropriate public instance variables is the case where the class is essentially a data structure, with no behavior. In other words, if you would have used a struct instead of a class (if Java supported struct), then it's appropriate to make the class's instance variables public. http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-137265.html#177

When used correctly, PODs (plain old data structures) are better than POJOs just like POJOs are often better than EJBs.
http://en.wikipedia.org/wiki/Plain_Old_Data_Structures

3

This kind of classes are quite useful when you are dealing with mid-size/big applications, for some reasons:

  • it's quite easy to create some test cases and ensure that data is consistent.
  • it holds all kind of behaviors that involve that information, so data bug tracking time is reduce
  • Using them should keep method args lightweight.
  • When using ORMs , this classes gives flexybility and consistency. Adding a complex attribute that's calculated based on simple information already in the class, resutls in writing one simple method. This is quite more agile and productive that having to check your database and ensure all databases are patched with new modification.

So to sum up, in my experience they usually are more useful than annoying.

3

With game design, the overhead of 1000's of function calls, and event listeners can sometimes make it worth it to have classes that only store data, and have other classes that loop through all the data only classes to perform the logic.

3

Structs have their place, even in Java. You should only use them if the following two things are true:

  • You just need to aggregate data that doesn't have any behavior, e.g. to pass as a parameter
  • It doesn't matter one bit what sort of values that aggregate data has

If this is the case, then you should make the fields public and skip the getters/setters. Getters and setters are clunky anyway, and Java is silly for not having properties like a useful language. Since your struct-like object shouldn't have any methods anyway, public fields make the most sense.

However, if either one of those do not apply, you're dealing with a real class. That means all fields should be private. (If you absolutely need a field at a more accessible scope, use a getter/setter.)

To check if your supposed-struct has behavior, look at when the fields are used. If it seems to violate tell, don't ask, then you need to move that behavior into your class.

If some of your data shouldn't change, then you need to make all those fields final. You might consider making your class immutable. If you need to validate your data, then provide validation in the setters and constructors. (A useful trick is to define a private setter and modify your field within your class using only that setter.)

Your Bottle example would most likely fail both tests. You could have (contrived) code that looks like this:

public double calculateVolumeAsCylinder(Bottle bottle) {
    return bottle.height * (bottle.diameter / 2.0) * Math.PI);
}

Instead it should be

double volume = bottle.calculateVolumeAsCylinder();

If you changed the height and diameter, would it be the same bottle? Probably not. Those should be final. Is a negative value ok for the diameter? Must your Bottle be taller than it is wide? Can the Cap be null? No? How are you validating this? Assume the client is either stupid or evil. (It's impossible to tell the difference.) You need to check these values.

This is what your new Bottle class might look like:

public class Bottle {

    private final int height, diameter;

    private Cap capType;

    public Bottle(final int height, final int diameter, final Cap capType) {
        if (diameter < 1) throw new IllegalArgumentException("diameter must be positive");
        if (height < diameter) throw new IllegalArgumentException("bottle must be taller than its diameter");

        setCapType(capType);
        this.height = height;
        this.diameter = diameter;
    }

    public double getVolumeAsCylinder() {
        return height * (diameter / 2.0) * Math.PI;
    }

    public void setCapType(final Cap capType) {
        if (capType == null) throw new NullPointerException("capType cannot be null");
        this.capType = capType;
    }

    // potentially more methods...

}
0

IMHO there often aren't enough classes like this in heavily object-oriented systems. I need to carefully qualify that.

Of course if the data fields have wide scope and visibility, that can be extremely undesirable if there are hundreds or thousands of places in your codebase tampering with such data. That's asking for trouble and difficulties maintaining invariants. Yet at the same time, that doesn't mean that every single class in the entire codebase benefits from information hiding.

But there are many cases where such data fields will have very narrow scope. A very straightforward example is a private Node class of a data structure. It can often simplify the code a great deal by reducing the number of object interactions going on if said Node could simply consist of raw data. That serves as a decoupling mechanism since the alternative version might require bidirectional coupling from, say, Tree->Node and Node->Tree as opposed to simply Tree->Node Data.

A more complex example would be entity-component systems as used often in game engines. In those cases, the components are often just raw data and classes like the one you've shown. However, their scope/visibility tends to be limited since there are typically only one or two systems that might access that particular type of component. As a result you still tend to find it quite easy to maintain invariants in those systems and, moreover, such systems have very few object->object interactions, making it very easy to comprehend what's going on at the bird's eye level.

In such cases you can end up with something more like this as far as interactions go (this diagram indicates interactions, not coupling, since a coupling diagram might include abstract interfaces for the second image below):

enter image description here

... as opposed to this:

enter image description here

... and the former type of system tends to be a lot easier to maintain and reason about in terms of correctness in spite of the fact that the dependencies are actually flowing towards data. You get much less coupling primarily because many things can be turned into raw data instead of objects interacting with each other forming a very complex graph of interactions.

-1

There are even much strange creatures exist in the OOP world. I've once created a class, which is abstract, and contains nothing, it just for to be a common parent of other two classes... it's the Port class:

SceneMember 
Message extends SceneMember
Port extends SceneMember
InputPort extends Port
OutputPort extends Port

So SceneMember is the base class, it does all the job, which required to appear on the scene: add, delete, get-set ID, etc. Message is the connection between ports, it has its own life. InputPort and OutputPort contain the i/o specific functions of their own.

Port is empty. It is just used to group InputPort and OutputPort together for, say, a portlist. It's empty because SceneMember contains all stuff which are for acting as a member, and InputPort and OutputPort contain the port-specified tasks. InputPort and OutputPort are such different, that they have no common (except SceneMember) functions. So, Port is empty. But it's legal.

Maybe it's an antipattern, but I like it.

(It's like the word "mate", which is used for both "wife" and "husband". You never use the "mate" word for a concrete person, because you know the sex of him/her, and it does not matter if he/she is married or not, so you use "someone" instead, but it still exists, and is used in rare, abstract situations, e.g. a law text.)

5
  • 1
    Why do your ports need to extend SceneMember? why not create the port classes to operate on SceneMembers?
    – Michael K
    Dec 27, 2010 at 15:11
  • 1
    So why not use the standard marker interface pattern? Basically identical to your empty abstract base class, but it's a much more common idiom.
    – TMN
    Dec 27, 2010 at 18:38
  • 1
    @Michael: Only for theoretical reasons, I reserved Port for future use, but this future hasn'nt arrived yet, and maybe never will. I didn't realised that they have no common at all, unlike their names suppose. I will pay compensation for everyone who suffered any loss occurred by that empty class...
    – ern0
    Dec 27, 2010 at 22:03
  • 1
    @TMN: SceneMember (derivative) types have getMemberType() method, there're several cases when Scene is scanned for subset of SceneMember objects.
    – ern0
    Dec 27, 2010 at 22:10
  • 4
    This does not answer the question.
    – Eva
    Jan 28, 2013 at 13:57

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