views:

278

answers:

8

Hi,

Unless I am thoroughly mistaken, the getter/setter pattern is a common pattern used for two things:

  1. To make a private variable so that it can be used, but never modified, by only providing a getVariable method (or, more rarely, only modifiable, by only providing a setVariable method).
  2. To make sure that, in the future, if you happen to have a problem to which a good solution would be simply to treat the variable before it goes in and/or out of the class, you can treat the variable by using an actual implementation on the getter and setter methods instead of simply returning or setting the values. That way, the change doesn't propagate to the rest of the code.

Question #1: Am I missing any use of accessors or are any of my assumptions incorrect? I'm not sure if I am correct on those.

Question #2: Are there any sort of template goodness that can keep me from having to write the accessors for my member variables? I didn't find any.

Question #3: Would the following class template be a good way of implementing a getter without having to actually write the accesor?

template <class T>
struct TemplateParameterIndirection // This hack works for MinGW's GCC 4.4.1, dunno others
{
    typedef T Type;
};

template <typename T,class Owner>
class Getter
{
public:
    friend class TemplateParameterIndirection<Owner>::Type; // Befriends template parameter

    template <typename ... Args>
    Getter(Args args) : value(args ...) {} // Uses C++0x

    T get() { return value; }

protected:
    T value;
};

class Window
{
public:
    Getter<uint32_t,Window> width;
    Getter<uint32_t,Window> height;

    void resize(uint32_t width,uint32_t height)
    {
        // do actual window resizing logic

        width.value = width; // access permitted: Getter befriends Window
        height.value = height; // same here
    }
};

void someExternalFunction()
{
    Window win;

    win.resize(640,480); // Ok: public method

    // This works: Getter::get() is public
    std::cout << "Current window size: " << win.width.get() << 'x' << win.height.get() << ".\n";

    // This doesn't work: Getter::value is private
    win.width.value = 640;
    win.height.value = 480;
}

It looks fair to me, and I could even reimplement the get logic by using some other partial template specialization trickery. The same can be applied to some sort of Setter or even GetterSetter class templates.

What are your thoughts?

A: 
  1. You can also use a getter or setter type method to get or set computable values, much the way properties are used in other languages like C#

  2. I can't think of reasonable way to abstract the getting and setting of an unknown number of values / properties.

  3. I'm not familiar enough with the C++ox standard to comment.

csj
The only part that uses the C++0x standard is the constructor. You can ignore it for now and comment on the rest. It was just something that made the variable constructable in a classes' constructor initializer list (i.e. `Window::Window() : width(640), height(480) {}`; the variadic arguments are "strong typedly" passed to the variable constructor).
n2liquid
I meant strongly typed*, sorry XD
n2liquid
Or maybe not.. Aww, *me english*..
n2liquid
A: 

this is where i think #defines are still useful.

The template version is complicated and hard to understand - the define version is obvious

#define Getter(t, n)\
     t n;\
     t get_##n() { return n; }

class Window
{
    Getter(int, height);
}

I am sure I have the syntax slightly wrong - but u get the point

if there was a well known set of templates in , say, boost then I would use them. But I would not write my own

pm100
For that to work correctly, I would need to write the variable declaration myself, because the Getter define you said would declare both members public. Also, there is the matter of naming: in my projects, no single define is in lowercase, and all defines begin prefixed by SONETTO_, in fact to make the creation of a new define a harder decision, only there for drastic purposes XD
n2liquid
I hate those macros
Viktor Sehr
i actually wrapped my post in 'flame on' 'flame off' in angle brackets but SO dropped them. I knew it would be controversial to say what I said; pragmatism vs purity. Everybody can see what that macro does, everybody can make changes to it, nobody except ultra c++ gurus understands the templated version
pm100
+1  A: 

FWIW here are my opinions on your questions:

  1. Typically the point is that there is business logic or other constraints enforced in the setter. You can also have calculated or virtual variables by decoupling the instance variable with accessor methods.
  2. Not that I know of. Projects I've worked on have had a family of C macros to stamp out such methods
  3. Yes; I think that's pretty neat. I just worry it's not worth the trouble, it'll just confuse other developers (one more concept they need to fit in their head) and isn't saving much over stamping out such methods manually.
Terry Mahaffey
A: 

This may be overkill in this case but you should check out the attorney/client idiom for judicious friendship usage. Before finding this idiom, I avoided friendship altogether.

http://www.ddj.com/cpp/184402053/

Jonathan
Also, you should drop 'Public' from the name 'PublicGetter'. It's accessibility is determined by the consumer.
Jonathan
Ah, you're right XD Don't know why I sticked with that "Public" thing, thanks.
n2liquid
+3  A: 

Whilst the solution is neat from implementation point of view, architectually, it's only halfway there. The point of the Getter/Setter pattern is to give the clas control over it's data and to decrease coupling (i.e. other class knowing how data is stored). This solution achieves the former but not quite the latter.

In fact the other class now has to know two things - the name of the variable and the method on the getter (i.e. .get()) instead of one - e.g. getWidth(). This causes increased coupling.

Having said all that, this is splitting proverbial architectural hairs. It doesn't matter all that much at the end of the day.

EDIT OK, now for shits and giggles, here is a version of the getter using operators, so you don't have to do .value or .get()

template <class T>
struct TemplateParameterIndirection // This hack works for MinGW's GCC 4.4.1, dunno others
{
    typedef T Type;
};

template <typename T,class Owner>
class Getter
{
public:
    friend TemplateParameterIndirection<Owner>::Type; // Befriends template parameter

    operator T()
    {
        return value;
    }

protected:
    T value;

    T& operator=( T other )
    {
       value = other;
       return value;  
    }


};

class Window
{
public:
    Getter<int,Window> _width;
    Getter<int,Window> _height;

    void resize(int width,int height)
    {
        // do actual window resizing logic
        _width = width; //using the operator
        _height = height; //using the operator
    }
};

void someExternalFunction()
{
    Window win;

    win.resize(640,480); // Ok: public method
    int w2 = win._width; //using the operator
    //win._height = 480; //KABOOM
}

EDIT Fixed hardcoded assignment operator. This should work reasonably well if the type itself has an assignment operator. By default structs have those so for simple ones it should work out of the box.

For more complex classes you will need to implement an assignment operator which is fair enough. With RVO and Copy On Write optimizations, this should be reasonably efficient at run time.

Igor Zevaka
I don't think I understand why `width.get()` would be any harder to know, understand or write than `getWidth()`. The only thing is people would at first need to check out what "is this Getter<> thing" to discover it always exposes a `get()` method. So, dunno.. Ignoring the decoupling part, is the trick worth in your opinion? I mean, it is working; I only have to decide whether to deploy it on my whole library or not.
n2liquid
As I said, it's a very marginal increase in coupling that is caught at compile time. I think it's OK to use. The point that I am trying to make is as far as other classes are concerned, it's a difference between **I get width from Window by calling getWidth** and **I get width from Window by calling method `get()` on member width**.
Igor Zevaka
Omg, that was the first thing I thought of when started to create the template, but I thought there wasn't an operator that would allow me to simply redirect the variable entirely to its member variable @__@ are you sure that works? I'll try here, but that would be too perfect to me...
n2liquid
compiles and runs (mcvc 9) :)
Igor Zevaka
Yes.. god, why didn't I think of that before. I really tried, but I have completely overseen implicit cast operators, and I have already used them before! Regarding the `operator=()`, I wanted to do that, but if the get was still needed, that would make the class even more unnatural. Now, this is perfect. It also compiled and worked here. Amazing. Thanks.
n2liquid
Sorry, this isn't actually perfect XD It doesn't work for structs and classes, as accessing a variable like if it was some type other than its own is never solved through implicit casting. Do you think you have a solution to that one? It would sure be amazing.
n2liquid
Igor Zevaka
Yeah, sure, but I changed that here and it works for other types. The problems really are only two: 1) the implicit cast isn't used when you use the "member" (.) and "member by pointer" (->) operators and 2) the implicit cast returns the value by copy, which is "ok" for the public interface that wouldn't need to modify it, but what about the very class? It couldn't modify the value of the variable without relying on accessing the `value` member anyway, so it breaks naturality.
n2liquid
And I think you're missing something: the problem with structs wasn't the assignment operator, but the actual access to the struct. `window.size.width`, being `window.size` a `Getter<Size,Window>`, and `Size` a struct like `struct Size { uint32_t width; }`, does not work, because the implicit cast isn't used by "." operator.
n2liquid
Hmmm yeah, it's a bit of a problem for `.` operator and `->` operator. You'll have to explicitly cast those getters to the underlying type to work with their members, which is cluttery at best. Like this: `((Size)window.size).DoSomethingOnTheStruct()`
Igor Zevaka
Yeah, and that would be way worst than simply get()'ing. Got no ideas? For pointers, I could overload -> to return a const pointer. For plain structs, though, I couldn't do a thing but using get(). And, besides, if I return const pointers or references, I would still have to resort to accessing `value` from inside the class friend itself. This kills.
n2liquid
Well if you cast to the underlying type, it will just work if the underlying type is a pointer, so you can do this `Getter<Struct*> foo; ((Struct*)foo)->do something`. One other thing is that `operator T()` should be `const`.
Igor Zevaka
It might actually be acceptable to add `T* operator -> (){return value;}. If T is an interface it'd be quite legitimate to add a non-const `->` operator.... Need a less trivial example to thrash it out.
Igor Zevaka
n2liquid
+1  A: 

Since Igor Zevaka posted one version of this, I'll post one I wrote a long time ago. This is slightly different -- I observed at the time that most real use of get/set pairs (that actually did anything) was to enforce the value of a variable staying within a pre-determined range. This is a bit more extensive, such as adding I/O operators, where extractor still enforces the defined range. It also has a bit of test/exercise code to show the general idea of what it does and how it does it:

#include <exception>
#include <iostream>
#include <functional>

template <class T, class less=std::less<T> >
class bounded {
    const T lower_, upper_;
    T val_;

    bool check(T const &value) {
        return less()(value, lower_) || less()(upper_, value);
    }

    void assign(T const &value) {
        if (check(value))
            throw std::domain_error("Out of Range");
        val_ = value;
    }

public:
    bounded(T const &lower, T const &upper) 
        : lower_(lower), upper_(upper) {}

    bounded(bounded const &init) 
        : lower_(init.lower), upper_(init.upper)
    { 
        assign(init); 
    }

    bounded &operator=(T const &v) { assign(v);  return *this; }

    operator T() const { return val_; }

    friend std::istream &operator>>(std::istream &is, bounded &b) {
        T temp;
        is >> temp;

        if (b.check(temp))
            is.setstate(std::ios::failbit);
        else
            b.val_ = temp;
        return is;
    }
};


#ifdef TEST

#include <iostream>
#include <sstream>

int main() {
    bounded<int> x(0, 512);

    try {
        x = 21;
        std::cout << x << std::endl;

        x = 1024;
        std::cout << x << std::endl;
    }

    catch(std::domain_error &e) {
        std::cerr << "Exception: " << e.what() << std::endl;
    }

    std::stringstream input("1 2048");
    while (input>>x)
        std::cout << x << std::endl; 

    return 0;
}

#endif
Jerry Coffin
A: 

And now the question, and what if you need a setter as well.

I don't know about you, but I tend to have (roughly) two types of classes:

  • class for the logic
  • blobs

The blobs are just loose collections of all the properties of a Business Object. For example a Person will have a surname, firstname, several addresses, several professions... so Person may not have logic.

For the blobs, I tend to use the canonical private attribute + getter + setter, since it abstracts the actual implementation from the client.

However, although your template (and its evolution by Igor Zeveka) are really nice, they do not address the setting problem and they do not address binary compatibility issues.

I guess I would probably resort to macros...

Something like:

// Interface
// Not how DEFINE does not repeat the type ;)
#define DECLARE_VALUE(Object, Type, Name, Seq) **Black Magic Here**
#define DEFINE_VALUE(Object, Name, Seq) ** Black Magic Here**

// Obvious macros
#define DECLARE_VALUER_GETTER(Type, Name, Seq)\
   public: boost::call_traits<Type>::const_reference Name() const

#define DEFINE_VALUE_GETTER(Object, Name)\
   boost::call_traits<Name##_type>::const_reference Object::Name ()const\
   { return m_##Name; }

#define DECLARE_VALUE_SETTER(Object, Type, Name)\
   public: Type& Name();\
   public: Object& Name(boost::call_traits<Type>::param_type i);

#define DEFINE_VALUE_SETTER(Object, Name)\
   Name##_type& Object::Name() { return m_##Name; }\
   Object& Object::Name(boost::call_traits<Name##_type>::param_type i)\
   { m_##Name = i; return *this; }

Which would be used like:

// window.h
DECLARE_VALUE(Window, int, width, (GETTER)(SETTER));

// window.cpp
DEFINE_VALUE(Window, width, (GETTER)); // setter needs a bit of logic

Window& Window::width(int i) // Always seems a waste not to return anything!
{ 
  if (i < 0) throw std::logic_error();
  m_width = i;
  return *this;
} // Window::width

With a bit of preprocessor magic it would work quite well!

#include <boost/preprocessor/seq/for_each.hpp>
#include <boost/preprocessor/tuple/rem.hpp>

#define DECLARE_VALUE_ITER(r, data, elem)\
  DECLARE_VALUE_##elem ( BOOST_PP_TUPLE_REM(3)(data) )

#define DEFINE_VALUE_ITER(r, data, elem)\
  DEFINE_VALUE_##elem ( BOOST_PP_TUPLE_REM(2)(data) )

#define DECLARE_VALUE(Object, Type, Name, Seq)\
   public: typedef Type Name##_type;\
   private: Type m_##Name;\
   BOOST_PP_SEQ_FOREACH(DECLARE_VALUE_ITER, (Object, Type, Name), Seq)

#define DEFINE_VALUE(Object, Name, Seq)\
   BOOST_PP_SEQ_FOREACH(DEFINE_VALUE_ITER, (Object, Name), Seq)

Okay, not type safe, and all, but:

  • it's a reasonable set of macro I think
  • it's easy to use, the user only ever have to worry about 2 macros after all, though like templates the errors could get hairy
  • use of boost.call_traits for efficiency (const& / value choice)
  • there is more functionality there: getter/setter duo

  • it is, unfortunately, a set of macros... and will not complain if you ever

  • it wreaks havoc on the accessors (public, protected, private) so it's best not to intersped it throughout the class

Here is the canonical example then:

class Window
{
  // Best get done with it
  DECLARE_VALUE(Window, int, width, (GETTER));
  DECLARE_VALUE(Window, int, height, (GETTER));

// don't know which is the current access level, so better define it
public:

};
Matthieu M.
+1  A: 

You're solving the wrong problem. In a well-designed application, getters and setters should be rare, not automated. A meaningful class provides some kind of abstraction. It is not simply a collection of members, it models a concept that is more than just the sum of its member variables. And it typically doesn't even make sense to expose individual members.

A class should expose the operations that make sense on the concept that it models. Most member variables are there to maintain this abstraction, to store the state that you need. But it typically shouldn't be accessed directly. That is why it is a private member of the class in the first place.

Rather than finding easier ways to write car.getFrontLeftWheel(), ask yourself why the user of the class would ever need the front left wheel in the first place. Do you usually manipulate that wheel directly when driving? The car is supposed to take care of all the wheel-spinning business for you, isn't it?

jalf
Well, speaking of encapsulation, members that are really, really private and nothing more than an implementation specific variable I use to place in a pointer-to-implementation structure (PImpl idiom). The screen resolution is definitely something you want to know about a window, or do you disagree? In a class with width, height, color depth, fullscreen, refresh rate, V-Sync, triple buffering and Direct3D objects variables (the class does not abstract those, it only resolves some issues regarding them), you can see that getters must be a common thing..
n2liquid