views:

474

answers:

10

Note: C++ specific question

I personally find it weird/ugly when a class uses a getter to access its own member data. I know the performance impact is none but I just don't like to see all those method calls. Are there any strong arguments either way, or is it just one of those things that's personal preference and should be left to each coder, or arbitrarily controlled in a coding standard?

Update: I'm meaning simple getters, specifically for a class' non-public members.

+9  A: 

The reason you might want to use a getter/setter is because it conceals the implementation. You won't have to rewrite all of your code if you are using getters/setters in case the implementation does change, because those members can continue to work.

EDIT based on the many clever comments:

As for a class using setters and getters on itself, that may depend on the particulars. After all, the implementation of a particular class is available to the class itself. In the cases where a class is normally instantiated, the class should use the member values directly for its own members (private or otherwise) and its parent classes (if they are protected) and only use getters/setters in the case that those members are private to the parent class.

In the case of an abstract type, which will usually not contain any implementation at all, it should provide pure virtual getters and setters and use only those in the methods it does implement.

TokenMacGuy
This is about a class itself using it's own getters/setters 'it's own member data'.
Chubsdad
...and the answer is also valid for a class using it's own getters/setters
stijn
But you could argue a class should _know_ how its private internal data is structured and should not want to conceal its implementation _from itself_
John
Using a field and then switching to a property could break any code that gets/sets that field trough reflection.
diamandiev
@diamandiev: AFAIK, C++ doesn't have "property" and doesn't support "reflection". If a language supports "property" but doesn't provide transparency between "property" and real "member variable", then that is a broken implementation of "property".
Lie Ryan
Well what I said could be useful in other programming languages. I didn't know C++ doesn't support reflection.
diamandiev
@diamandiev: Most compiled languages, e.g. C++, doesn't have enough runtime information to do reflection. If you really need to do reflection, you'll need to do some extra work to provide the runtime information, or ask the compiler to compile in the information.
Lie Ryan
This is why my title included C++ to stop it getting more general, but someone edited it... I _knew_ people would jump in without noticing it was C++ - specific.
John
I think it's perfectly acceptable to access your own private values directly. It's faster and potentially avoids inline bloat. However if your class is abstract then it is probably good practice to provide getters to prevent subclasses or clients from taking the same shortcuts.
locka
Can you back up your claims about "faster" and "inline bloat"? Surely `Type getSomething(){ return something; }` is going to be trivially replaced with `something` on any compiler this century?
John
You assume the getter is just a simple wrapper that returns a method. The class may wish to log access to the value, or do some kind of sanity checking, all of which will be duplicated if you inline. And if the getter is not inlined then you incur a function call. While it's entirely possible for a class to use a getter for itself, I expect in most instances it is terser and faster to just reference the value directly.
locka
+2  A: 

An argument in favor of using getters is that you might decide one day to change how the member field is calculated. You may decide that you need it to be qualified with some other member, for instance. If you used a getter, all you have to do is change that one getter function. If you didn't you have to change each and every place where that field is used currently and in the future.

Nathan Fellman
+3  A: 

The only advantage is that it allows changing internal representation without changing external interface, permitting lazy evaluation, or why not access counting.

In my experience, the number of times I did this is very, very low. And it seems you do, I also prefer to avoid the uglyness and weightyness of getter/setters. It is not that difficult to change it afterwards if I really need it.

As you speak about a class using its own getter/setters in its own implementation functions, then you should consider writing non-friend non-member functions where possible. They improve encapsulation as explained here.

Didier Trosset
I'm talking about a class internally using its own members, which might be protected/private
John
+1  A: 

Protecting a member variable by wrapping its access with get/set functions has its advantages. One day you may wish to make your class thread-safe - and in that instance, you'll thank yourself for using those get/set functions

freefallr
Wouldn't it be enough to make the public API methods thread-safe?
John
yes, I thought that those get/set functions would be part of the public API
freefallr
please... don't make your class thread-safe by making every single access thread safe. That doesn't help a bit!
xtofl
@xtofl: it may help a lot - depending on the class - but I know what you mean :-). As well as typically not actually working (in light of iterator invalidation, multi-field transactional needs etc.), it can easily lead to dead-lock or inefficient recursive mutex abuse.
Tony
+2  A: 

Just a crude example. Does this help?

struct myclass{
    int buf[10];
    int getAt(int i){
        if(i >= 0 && i < sizeof(buf)){
            return buf[i];
        }
    }

    void g(){
        int index = 0;
        // some logic
        // Is it worth repeating the check here (what getAt does) to ensure
              // index is within limits
        int val = buf[index];
    }
};

    int main(){}

EDIT:

I would say that it depends. In case the getters do some kind of validation, it is better to go through the validation even if it means the class members being subjected to that validation. Another case where going through a common entry point could be helpful is when the access needs to be essentially in a sequential and synchronized manner e.g. in a multithreaded scenario.

Chubsdad
That's _not_ a getter
John
what's a getter in your opinion?
Chubsdad
A getter/setter is normally defined as a method to get/set the value of a variable... your example is more like an indexer or something.
John
@Chubsdad: i.e. `int get_n() const { return n_; } This return *this; }` and using these rather than n_ directly.
Tony
@John: Chubsdad's example is a getter because it abstracts the process of fetching a value. An 'indexer' is a specific type of getter. If, for example, you switch to using vector<int> instead of int[] array, the user of your getter (which might be yourself) does not need to know about that.
Lie Ryan
@Lie: actually, it's void and takes no arguments. But let's say it was returning "val" - then it would be a getter, but still not the kind of trivial get/set example for which someone might argue over whether to use it or directly access a member, as if you need the calculated value from `g()` then you're not even going to consider replicating all the code inside it. Definitely an example of why a getter might end up being good - because you might have started with a trivial getter and want to move to something like `int g() const`.
Tony
@Tony: I'm referring to `int getAt(int i)` when I'm saying it's a getter. In Chubsdad's example, `g()` is the user of the getter method named `getAt(i)`.
Lie Ryan
I'm not convinced any book would say a method passed an argument is a classical 'getter'.
John
@John: If a book specifically said it's not a getter, then I can safely assume that book is trash.
Lie Ryan
Good counter-argument. Can you back it up with _facts_, rather than your _personal_ use of the word, because I would certainly say a no-argument getter is what _most_ people understand to _be_ a getter. If you can I'm happy to see it, but the onus is on you to substantiate your claim rather than make spurious statements about unnamed books.
John
+9  A: 

Willingness to use getters/setters within class member implementation is the canary in the mine telling that your class is growing unreasonably. It tells that your class is trying to do too many different things, that it serves several purposes where it should serve one instead.

In fact, this is usually encountered when you are using one part of your class to store or access your data, and another part to make operations on it. Maybe you should consider using a standalone class to store and give access to your data, and another one to provide a higher view, with more complex operations with your data.

Didier Trosset
It's not a very accurate OO picture of the entity you're trying to model, if it and its data are separate classes. Seems like over-design to me, and besides what about a class `Person` with member `name`, it's hardly a sign the class is too big if internally I use `getName()` over `name`.
John
It *is* a sign. If you really think about using `getName()` instead of `name` withing the class implementation, then it means that you think you may get the name from somewhere else. Getting the name *is* another feature, not the actual class feature.
Didier Trosset
+1 I agree 100%, especially if you have a lot of setters.
Viktor Sehr
This is a good point - needing encapsulation walls within a single class does indeed suggest the class is too complex, though there are other considerations.
Tony
calling `getName()` doesn't mean I might expect the name to come from elsewhere. You're treating your thought process like a global rule.
John
If you don't expect the name to came from somewhere esle, if you know that the name is present, then use it. Don't call it from somewhere using a function.
Didier Trosset
We've gone all the way round then. I don't agree with your argument, but I _think_ we agree that a class accessing its own private members directly is fine/preferable.
John
Yes, I confirm we agree on this.
Didier Trosset
+2  A: 

this is actually for supporting the object oriented-ness of the class by abstracting the way to get(getter). and just providing its easier access.

CadetNumber1
+3  A: 

It seems most people didn't read your question properly, the question is concerning whether or not class methods accessing its own class' members should use getters and setters; not about an external entity accessing the class' members.

I wouldn't bother using getter and setter for accessing a class' own members.

However, I also keep my classes small (typically about 200-500 lines), such that if I do need to change the fields or change its implementations or how they are calculated, search and replace wouldn't be too much work (indeed, I often change variable/class/function names in the early development period, I'm picky name chooser).

I only use getter and setters for accessing my own class members when I am expecting to change the implementation in the near future (e.g. if I'm writing a suboptimal code that can be written quickly, but plans to optimize it in the future) that might involve radically changing the data structure used. Conversely, I don't use getter and setter before I already have the plan; in particular, I don't use getter and setter in expectation of changing things I'm very likely never going to change anyway.

For external interface though, I strictly adhere to the public interface; all variables are private, and I avoid friend except for operator overloads; I use protected members conservatively and they are considered a public interface. However, even for public interface, I usually still avoid having direct getters and setters methods, as they are often indicative of bad OO design (every OO programmers in any language should read: Why getter and setter methods are Evil). Instead, I have methods that does something useful, instead of just fetching the values. For example:

class Rectangle {
    private:
        int x, y, width, height;
    public:
        // avoid getX, setX, getY, setY, getWidth, setWidth, getHeight, setHeight
        void move(int new_x, int new_y);
        void resize(int new_width, int new_height);
        int area();
}
Lie Ryan
Hey - I read the question properly! ;-P.
Tony
@Tony: Your answer comes after I started writing the reply, but before I press post. I agree though, that the statement is too broad a sweeping statement.
Lie Ryan
+6  A: 

THE OBVIOUS

getters and setters for protected members makes as much sense as for public... derived classes are just another form of client code, and encapsulating implementation details from them can still be useful. I'm not saying always do it, just to weight pros and cons along the normal lines.

getters and setters for private members is rarely a net benefit, though:

  • it does provide the same kind of encapsulation benefits

    • single place for breakpoints/logging of get/set + invariant checks during dev (if used consistently)
    • virtual potential
    • etc...

    but only to the presumably relatively small implementation of the same struct/class. In enterprise environments, and for public/protected member data, those benefits can be substantial enough to justify get/set methods: a logging function may end up having millions of lines of code depedent on it, and hundreds or thousands of libraries and apps for which a change to a header may trigger recompilation. Generally a single class implementation shouldn't be more than a few hundred (or at worst thousand) lines - not big or complex enough to justify encapsulating internal private data like this... it could be said to constitute a "code smell".

THE NOT-SO OBVIOUS

  • get/set methods can very occasionally be more readable than direct variable access (though more often less readable)
  • get/set methods may be able to provide a more uniform and convenient interface for code-generated member or friend methods (whether from macros or external tools/scripts)
  • less work required to transition between being a member or friend to a freestanding helper function should that become possible
  • implementation may be rendered more understandable (and hence maintainable) to people who're normally only users of the class (as more operations are expressed via, or in the style of, the public interface)

It's a bit out of scope for the question, but it's worth noting that classes should generally provide action-oriented commands, event-triggered callbacks etc. rather than encouraging a get/set usage pattern.

Tony
A decent debugger will allow you to set a 'watch', so the debugger will pause if the value of the watched variable is changed/accessed, you don't need a getter/setter for that.
Lie Ryan
@Lie: yes - had considered that. Interfactive debuggers are great for some types of debugging, but examing trace can be much more productive for other types of trouble-shooting, as well as for elapsed-time profiling, flow analysis, overall member-usage analysis, and lots of other creative purposes.
Tony
@Tony: I pity the fool who uses public/protected data as part of a class since it prevents any class invariant preservation (note: there isn't a single difference between the two levels as far as data is concern... save from a mere hint to the developer).
Matthieu M.
+1, I like the last sentence.
Alexandre C.
+1  A: 

Simple answer. If you are writing a one shoot program, that will never change, you can leave the getters at peace and do without any.

However if you write a program that could change or been written over time, or others might use that code, use getters.

If you use getters it helps change the code faster later on, like putting a guard on the property to verify correctness of value, or counting access to the property(debugging).

Getters to me are about easy possibilities(free lunch). The programmer who write the code does not need getters, he wants them.

hope that help.

none