Isn't there a point where encapsulation gets ridiculous?

2.8k views Asked by At

For my software development programming class we were supposed to make a "Feed Manager" type program for RSS feeds. Here is how I handled the implementation of FeedItems.

Nice and simple:

struct FeedItem {
    string title;
    string description;
    string url;
}

I got marked down for that, the "correct" example answer is as follows:

class FeedItem
{
public:
    FeedItem(string title, string description, string url);

    inline string getTitle() const { return this->title; }
    inline string getDescription() const { return this->description; }
    inline string getURL() const { return this->url; }

    inline void setTitle(string title) { this->title = title; }
    inline void setDescription(string description){ this->description = description; }
    inline void setURL(string url) { this->url = url; }

private:
    string title;
    string description;
    string url;
};

Now to me, this seems stupid. I honestly can't believe I got marked down, when this does the exact same thing that mine does with a lot more overhead.


It reminds me of how in C# people always do this:

public class Example
{
    private int _myint;

    public int MyInt
    {
        get
        {
            return this._myint;
        }
        set
        {
            this._myint = value;
        }
    }
}

I mean I GET why they do it, maybe later on they want to validate the data in the setter or increment it in the getter. But why don't you people just do THIS UNTIL that situation arises?

public class Example
{
    public int MyInt;
}

Sorry this is kind of a rant and not really a question, but the redundancy is maddening to me. Why are getters and setters so loved, when they are unneeded?

13

There are 13 answers

6
Chris Thompson On BEST ANSWER

It's an issue of "best practice" and style.

  • You don't ever want to expose your data members directly. You always want to be able to control how they are accessed. I agree, in this instance, it seems a bit ridiculous, but it is intended to teach you that style so you get used to it.
  • It helps to define a consistent interface for classes. You always know how to get to something --> calling its get method.

Then there's also the reusability issue. Say, down the road, you need to change what happens when somebody accesses a data member. You can do that without forcing clients to recompile code. You can simply change the method in the class and guarantee that the new logic is utilized.

4
ChssPly76 On

Here's a nice long SO discussion on the subject: Why use getters and setters.

The question you want to ask yourself is "What's going to happen 3 months from now when you realize that FeedItem.url does need to be validated but it's already referenced directly from 287 other classes?"

2
Reed Copsey On

The main reason to do this before its needed is for versioning.

Fields behave differently than properties, especially when using them as an lvalue (where it's often not allowed, especially in C#). Also, if you need to, later, add property get/set routines, you'll break your API - users of your class will need to rewrite their code to use the new version.

It's much safer to do this up front.

C# 3, btw, makes this easier:

public class Example
{
    public int MyInt { get; set; }
}
4
Heath Hunnicutt On

I agree with you, but it's important to survive the system. While in school, pretend to agree. In other words, being marked down is detrimental to you and it is not worth it to be marked down for your principles, opinions, or values.

Also, while working on a team or at an employer, pretend to agree. Later, start your own business and do it your way. While you try the ways of others, be calmly open-minded toward them -- you may find that these experiences re-shape your views.

Encapsulation is theoretically useful in case the internal implementation ever changes. For example, if the per-object URL became a calculated result rather than a stored value, then the getUrl() encapsulation would continue to work. But I suspect you already have heard this side of it.

1
wilhelmtell On

I absolutely agree with you. But in life you should probably do The Right Thing: in school, it's to get good marks. In your workplace it's to fulfill specs. If you want to be stubborn, then that's fine, but do explain yourself -- cover your bases in comments to minimize the damage you might get.

In your particular example above I can see you might want to validate, say, the URL. Maybe you'd even want to sanitize the title and the description, but either way I think this is the sort of thing you can tell early on in the class design. State your intentions and your rationale in comments. If you don't need validation then you don't need a getter and setter, you're absolutely right.

Simplicity pays, it's a valuable feature. Never do anything religiously.

0
StackedCrooked On

As a C++ developer I make my members always private simply to be consistent. So I always know that I need to type p.x(), and not p.x.

Also, I usually avoid implementing setter methods. Instead of changing an object I create a new one:

p = Point(p.x(), p.y() + 1);

This preserves encapsulation as well.

1
Doug T. On

There absolutely is a point where encapsulation becomes ridiculous.

The more abstraction that is introduced into code the greater your up-front education, learning-curve cost will be.

Everyone who knows C can debug a horribly written 1000 line function that uses just the basic language C standard library. Not everyone can debug the framework you've invented. Every introduced level encapsulation/abstraction must be weighed against the cost. That's not to say its not worth it, but as always you have to find the optimal balance for your situation.

2
WW. On

Maybe both options are a bit wrong, because neither version of the class has any behaviour. It's hard to comment further without more context.

See http://www.pragprog.com/articles/tell-dont-ask

Now lets imagine that your FeedItem class has become wonderfully popular and is being used by projects all over the place. You decide you need (as other answers have suggested) validate the URL that has been provided.

Happy days, you have written a setter for the URL. You edit this, validate the URL and throw an exception if it is invalid. You release your new version of the class and everyone one using it is happy. (Let's ignored checked vs unchecked exceptions to keep this on-track).

Except, then you get a call from an angry developer. They were reading a list of feeditems from a file when their application starts up. And now, if someone makes a little mistake in the configuration file your new exception is thrown and the whole system doesn't start up, just because one frigging feed item was wrong!

You may have kept the method signature the same, but you have changed the semantics of the interface and so it breaks dependant code. Now, you can either take the high-ground and tell them to re-write their program right or you humbly add setURLAndValidate.

2
Vincent Ramdhanie On

One of the problems that the software industry faces is the problem of reusable code. Its a big problem. In the hardware world, hardware components are designed once, then the design is reused later when you buy the components and put them together to make new things.

In the software world every time we need a component we design it again and again. Its very wasteful.

Encapsulation was proposed as a technique for ensuring that modules that are created are reusable. That is, there is a clearly defined interface that abstracts the details of the module and make it easier to use that module later. The interface also prevents misuse of the object.

The simple classes that you build in class do not adequately illustrate the need for the well defined interface. Saying "But why don't you people just do THIS UNTIL that situation arises?" will not work in real life. What you are learning in you software engineering course is to engineer software that other programmers will be able to use. Consider that the creators of libraries such as provided by the .net framework and the Java API absolutely require this discipline. If they decided that encapsulation was too much trouble these environments would be almost impossible to work with.

Following these guidelines will result in high quality code in the future. Code that adds value to the field because more than just yourself will benefit from it.

One last point, encapsulation also makes it possible to adequately test a module and be resonably sure that it works. Without encapsulation, testing and verification of your code would be that much more difficult.

1
dso On

Keep in mind that coding "best practices" are often made obsolete by advances in programming languages.

For example, in C# the getter/setter concept has been baked into the language in the form of properties. C# 3.0 made this easier with the introduction of automatic properties, where the compiler automatically generates the getter/setter for you. C# 3.0 also introduced object initializers, which means that in most cases you no longer need to declare constructors which simply initialize properties.

So the canonical C# way to do what you're doing would look like this:

class FeedItem
{
    public string Title { get; set; } // automatic properties
    public string Description { get; set; }
    public string Url { get; set; }
};

And the usage would look like this (using object initializer):

FeedItem fi = new FeedItem() { Title = "Some Title", Description = "Some Description", Url = "Some Url" };

The point is that you should try and learn what the best practice or canonical way of doing things are for the particular language you are using, and not simply copy old habits which no longer make sense.

2
Charles Eli Cheese On

If something's a simple struct, then yes it's ridiculous because it's just DATA.

This is really just a throwback to the beginning of OOP where people still didn't get the idea of classes at all. There's no reason to have hundreds of get and set methods just in case you might change getId() to be an remote call to the hubble telescope some day.

You really want that functionality at the TOP level, at the bottom it's worthless. IE you would have a complex method that was sent a pure virtual class to work on, guaranteeing it can still work no matter what happens below. Just placing it randomly in every struct is a joke, and it should never be done for a POD.

2
Jay On

Getters/Setters are, of course, good practice but they are tedious to write and, even worse, to read.

How many times have we read a class with half a dozen member variables and accompanying getters/setters, each with the full hog @param/@return HTML encoded, famously useless comment like 'get the value of X', 'set the value of X', 'get the value of Y', 'set the value of Y', 'get the value of Z', 'set the value of Zzzzzzzzzzzzz. thump!

0
A-K On

This is a very common question: "But why don't you people just do THIS UNTIL that situation arises?". The reason is simple: usually it is much cheaper not to fix/retest/redeploy it later, but to do it right the first time. Old estimates say that maintenance costs are 80%, and much of that maintenance is exactly what you are suggesting: doing the right thing only after someone had a problem. Doing it right the first time allows us to concentrate on more interesting things and to be more productive.

Sloppy coding is usually very unprofitable - your customers are unhappy because the product is unreliable and they are not productive when the are using it. Developers are not happy either - they spend 80% of time doing patches, which is boring. Eventually you can end up losing both customers and good developers.