Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
1  
This doesn't quite answer your question but seems relevant nonetheless: stackoverflow.com/questions/36701/struct-like-objects-in-java –  Anna Lear Dec 27 '10 at 14:39
1  
I think the term you are looking for is POD (Plain Old Data). –  Gaurav Dec 27 '10 at 14:51
7  
This is a typical example of structured programming. Not necessarily bad, just not object oriented. –  Konrad Rudolph Dec 27 '10 at 15:02
1  
shouldn't this be on stack overflow? –  Muad'Dib Dec 27 '10 at 15:42
4  
@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 '10 at 18:42

9 Answers 9

up vote 42 down vote accepted

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.

share|improve this answer
3  
True. And it would make it easier to add methods to Bottle later if you need to, right? –  Michael K Dec 27 '10 at 15:02
12  
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. –  Konrad Rudolph Dec 27 '10 at 16:00
5  
@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. –  Konrad Rudolph Dec 27 '10 at 16:09
8  
@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.) –  Konrad Rudolph Dec 27 '10 at 16:53
6  
@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. –  Anna Lear Dec 27 '10 at 17:12

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.

share|improve this answer
    
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 '12 at 17:21

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.

share|improve this answer
8  
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 '10 at 20:35
4  
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 '10 at 22:04
1  
@Thomas: I'm not saying there aren't times when you could have code maliciously misusing code you've written, but it seems that you shouldn't spend all of your time acting like your code other code is out to attack the code you're working on at the moment. Ultimately, you have to trust your code. If you can't trust it, fix it. Perhaps we should have debugging tools that keep track of information (like the stack trace) about where the current value of a variable came from, and maybe the same information about previous values of a variable. Thoughts anyone? –  compman Dec 28 '10 at 2:08
2  
@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 '10 at 4:29
6  
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 –  Peter Lawrey Dec 30 '10 at 1:08

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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...

}
share|improve this answer

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.)

share|improve this answer
1  
Why do your ports need to extend SceneMember? why not create the port classes to operate on SceneMembers? –  Michael K Dec 27 '10 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 '10 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 '10 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 '10 at 22:10
2  
This does not answer the question. –  Eva Jan 28 '13 at 13:57

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

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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