Isn't there a point where encapsulation gets ridiculous?

Question

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?

Hypabyssal

If you think this is ridiculous, you haven't used a library with a singleton Factory class that has one method that returns a single Manager instance with a single method that returns Mapper classes that return classes with Properties that contain strings of SQL statements. Oh, and each intermediate class has an Interface. – Epimenides

If you do it your way until it needs to be changed it is already too late. You have a member title (that is a string). But sometime down the road we need to change the implementation to a char array. Then unfortunately your implementation is stuck (everybody is already using title (and its part of the public API) and you can't get them to change so you can't change your implementation. If you hide the implementation behind a set method then how you implement the private part is not exposed and thus can be changed. – Fetishist

Personally I hate geters/setters (Its Java like and not C++ like). Why are you fiddling with the internals of the object. It does not look like it makes sense to alter these members once created (but this depends on the usage requirements). I would make all the members private, constructor to create them and see how the object is used. Adding unnesacery getters/setters is nearly as bad as having a public variable. – Fetishist

@Martin - I think the "API is already public, so it's too late" argument is a false one that gets used too frequently. Sure, if all you want to change is the internal implementation mechanism, then the getters/setters may protect clients from change. But if you want to change the behavior at all, pretending that getters/setters protect users from change is a lie. If setX() can now fail (say, due to access control) where it never failed before, the interface to the class has changed, and users will break anyways. It's even worse, because it will break at runtime instead of compile-time. – Quadrant

In C# I'd write public class Example{ public int MyInt{get;set;} } and that's not much longer than a field. – Futures

A note regarding C#: public int MyInt; is a class member which is not the same as public int MyInt{get;set;} which is a property. If you later need to substitute the member with a property, you will be changing the signature of your class. Code previously pointing to the member will not know that the identically named property was intended. The difference is that in IL changing a member is changing a variable but changing a property is calling a method. This is one of the few times that this otherwise beautiful syntactic sugar can bite you. – Adjustment

Answer 1

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.

Climax

Yes I understand that, and the standard interface is a great argument. But consider the C# example, changing between the two wouldn't be difficult at all if the need comes up, and the interface remains the same. And in my C++ example, FeedItem would never require any more overhead, it's just a convenient way to hold some data, not to act on it at all. – Hypabyssal

Perhaps with regards to your C# example, however with your C++ example, you're making assumptions. The key to good software engineering is designing your code so that you can easily change it when the assumptions change. Even though it may seem a bit ridiculous, and honestly, I agree with you, in this case, probably not necessary, what assumptions about your code could change? Maybe the urls are provided to you in a relative manner but your code assumes they are absolute, changing this class is a heck of a lot easier than changing the 3k programs that use it :-) – Climax

It's usually not that big a problem. If you really need to change what happens when somebody accesses a data member you can still change the class interface to make it a private member and provide a getter method. The compiler will then point out where you need to modify the rest of your code. Not so much work. – Geography

No, the interface in the C# example doesn't remain the same. You'd be changing a field to a property. All clients would need to be recompiled. This may be less of an issue in C++ where code is typically compiled and linked into one binary, but in the .NET environment where assemblies get "linked" at runtime, it's quite important. – Klemens

@ StackedCrooked: Sure if all the code is yours. But what happens to the 13000 applications that use your interface. Are you going to go around a change everybody else code? Even if you could other groups may object to this as there code is in lock down or requires a massive test release cycle (for legal or other reasons). – Fetishist

You can do that without forcing clients to recompile code of course ant it would be sweet however i dont think i was ever done in the endustry. – Evade

Answer 2

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?"

Prentice

Well in that particular case I was validating the URL before storing it. The struct was just a convenient way to hold the data, not to really act on it from within. Thanks for that link too btw. – Hypabyssal

You should think about (in an object-oriented way) who (which class) is responsible for validating a URL. I think it makes sense for the FeedItem class to validate the URL as it is the only class that can guarantee that the URL will always be valid. – Lunitidal

Everyone saying to validate URL misses the point of an RSS feed. There's no need to validate the URL!!!!!!!!!! You just display what the feed says, if the URL is invalid that's on the feed publisher not on me. – Hypabyssal

However, wouldn't such a radical change essentially change the contract of the function? 287 other classes would still compile, but now FeedItem does something quite different for them? E.g, should any error handling be added? – Behre

Answer 3

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; }
}

Eisteddfod

Indeed. In C# it is now just as easy to create a property as it is to create a public field, so may as well do it right. – Allman

However in C++ you actually can write properties, fully usable as lvalues. See here an example. – Sitzmark

Answer 4

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.

Honorine

Sometimes this is a significant factor in making a decision about a class design. Just be conscious about it. Sometimes it's just wrong, and this Roman way of doing things is the reason why things are getting out of hands. Then you simplify things and explain yourself. Someone might hit you with a bat and you'll have to correct yourself, but I think this whole process was still healthy for the system. – Honorine

Answer 5

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.

El

But in a proper OOP approach, you'd probably never use a struct. You seem to think it's rare that extra code is needed to access members, but that's not true. Bottom line, your IDE can auto-generate default getters/setters and the compiler can optimize them out. Writing this comment took longer than it would to auto-create getters/setters on 10 classes. – Frenchy

The problem with "proper OOP approach" is that not all data are objects. Sometimes they're just... pieces of data. Some bit of information, that has no behavior by itself, but gets used by other parts of the system. And if it isn't a scalar value, it has to be some sort of aggregate structure, which places us at a POD struct. – Quadrant

Answer 6

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.

Cahra

Great point: if this is just a POD class, the value of encapsulation is a great deal less. – Klemens

Maybe, rather than being held as a string, the URL itself should be an instance of a class that validates its own content so that neither the FeedItem nor any client of that class would need to validate it. If they've been given a URL instance then they know that they can rely on it being valid as it would have been checked when that instance was constructed. That's how I would approach it - a URL/URI isn't just a POD, it has strict syntax and might offer such methods as getScheme(), getHostName(), getQueryString(), etc. – Willing

Answer 7

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.

Embroideress

+1. It's a huge pity though that C++ doesn't have properties syntax in language (they can be emulated with some templates, but still) and Bjarne seems to just reject completely the need in it. – Sitzmark

Answer 8

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.

Geography

Answer 9

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.

Pulpit

Avoiding unneeded complexity is an interesting argument, but it doesn't stand up against the objection that getters and setters present a very common interface and virtually any programmer is familiar with it. – Sitzmark

Answer 10

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.

Anaya

This seems to be an anti-prototyping view, though. – Fronniah

@Tchalvak Not necessarily. The objective is to achieve well encapsulated classes. There is no reason why this cannot be done in increments. – Anaya

Answer 11

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!

Mail

You don't need to write them. Get a decent IDE, or a plugin. – Frenchy

@John: If your getters and setters are so dumb that your IDE can write them for you, you're just proving the original poster's point. The whole concept of explicit getters and setters is fundamentally flawed and needs to be purged from software engineering as soon as humanly possible. – Mordant

Answer 12

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.

Leilanileininger

Answer 13

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.

Brooklet

"While in school, pretend to agree. Also while working... pretend to agree. Later, start your own business and do it your way" The next sentence should have been "... and then you'll find out why everyone else is doing it the other way." – Herbst

Well, I'm not interested in being snarky. It's possible the OP has a point about encapsulation sometimes being carried too far. – Brooklet

Heartily disagree. If you are working with a professor who would mark you down for -discussing- your view of the benefits of simplicity, find a different professor. If you are working with a employer who isn't willing to entertain the benefits of simplicity, find an employer who isn't going to lead you down a tangled path to debugging. Just be sure that YOU are always ready to find out that -you're- wrong, from the discussion. – Fronniah

In this case, it seems the student handed in an assignment which didn't fit the teachings of the class. If the professor rejected discussion, that is objectionable. But in the case of turned-in written assignments in the context of a specific class, but which affect your future, "go along with the instructor's preferences" might be a better way for me to have put it. – Brooklet