7

I've got a struct "margin" in my class with 4 properties. Instead of writing four different getter/setter methods, I figured I could do it in a better way:

class myClass {
    private:
        struct margin {
            int bottom;
            int left;
            int right;
            int top;
        }
    public:
        struct getMargin();
        void setMargin(string which, int value);
};

But how can I set the property of the struct corresponding with the string "which" from within the function setMargin()? For example, if I call myClass::setMargin("left", 3), how can I then set "margin.left" to "3"? Preferably while keeping the struct POD? I really can't figure this out...

And on a side note, is this really better than writing many getter/setter methods?

Thanks!

Community
  • 1
  • 1
Frog
  • 1,631
  • 2
  • 17
  • 26
  • 2
    *"And on a side note, is this really better than writing many getter/setter methods?"* - Nope, it is much, much worse. There is nothing wrong with writing get/set methods *when they are needed*. They are not always required. – Ed S. Aug 13 '12 at 18:53
  • @EdS. But are they needed in this case? It would mean writing 8 methods instead of 2, without a clear advantage (it seems to me). Or is there an advantage? – Frog Aug 13 '12 at 18:56
  • 1
    So, let's be clear; typing strings to variable names is a horrible, horrible idea. Just throw that one out. Per your question ("are they needed"), well, I don't know; it depends on your application. Do you require any sort of validation on the properties? If the answer is unequivocally "no" then no, you don't need to wrap them at all. – Ed S. Aug 13 '12 at 18:58
  • +1 for being challenging question for me ^^ – huseyin tugrul buyukisik Aug 13 '12 at 19:00

5 Answers5

10

First, your idea is terrible... :)

Note you don't even have a margin member (added below)

I'd use an enum for this, if you don't want setters/getters for each property:

class myClass {
    private:
        struct margin {
            int bottom;
            int left;
            int right;
            int top;
        } m;  // <--- note member variable
    public:
        enum Side
        {
           bottom, left, rigth, top
        };
        struct getMargin();
        void setMargin(Side which, int value);
};

and have a switch statement inside setMargin.

void myClass::setMargin(Side which, int value)
{
    switch (which)
    {
        case bottom:
           m.bottom = value;
           break;
    //....
    }
}
Brian Webster
  • 30,033
  • 48
  • 152
  • 225
Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • But you also suggest writing getter and setter methods? – Frog Aug 13 '12 at 18:58
  • @Frog I think that's just a matter of preference. Now that I think about it, I also prefer the single method with the enum instead of the setters/getters. – Luchian Grigore Aug 13 '12 at 18:59
  • why use a margin struct instead of an int[4] ? then you wont need a switch – Gir Aug 13 '12 at 19:02
  • 1
    @Gir IDK how to answer that. OOP principles? The fact that this is C++ and not C? Because it's cleaner? Encapsulation? Single responsibility?... – Luchian Grigore Aug 13 '12 at 19:04
  • 1
    Encapsulation only matters in from the outside. As an implementation detail an `int[4]` would be fine. Then the switch is gone; `sides[which] = value`;`. That said, you lose some validation there and may want to check for valid ranges, so meh. – Ed S. Aug 13 '12 at 19:05
  • @EdS. I don't know about that. You could say the outer class is outside (which it is). Depends on the purpose of `margin` I guess. – Luchian Grigore Aug 13 '12 at 19:07
  • I suppose it just depends on how it is used. Personally I would go for your route over `int[4]`, but I don't see a huge problem with it either. – Ed S. Aug 13 '12 at 19:15
  • @Gir Using a struct sounded cleaner to me and this way I also don't have to worry about ranges. – Frog Aug 13 '12 at 19:16
3
class myClass {
private:
    int margin[4];
public:
    enum Side
    {
       bottom, left, rigth, top
    };
    void setMargin(Side which, int value);
};

void myClass::setMargin(Side which, int value)
{
    margin[which]=value;
}
Gir
  • 839
  • 5
  • 11
1

Either Luchian's or Gir's suggestion would be my preference. If you really want need a look up by a string, though, it is probably best to do that with an associative container.

class MyClass {
    std::map<std::string, int> m_;
public:
    bool setMargin(std::string which, int value) {
        std::map<std::string, int>::iterator i = m_.find(which);
        if (i == m_.end()) return false;
        i->second = value;
        return true;
    }
};

This is only useful if you have a dynamic interface that allows your user to define their own margins by name.

jxh
  • 69,070
  • 8
  • 110
  • 193
  • Not what I need, but a great solution! – Frog Aug 13 '12 at 21:01
  • 1
    It will take hours to find the annoying bug produced by a simple typo: `setMargin("rigth", 5)` – quinmars Aug 13 '12 at 21:04
  • @quinmars: As I stated in my answer, it is only useful as a data structure for a dynamic interface, not as an API. If the user of an editor was allowed to dynamically create margins and name them, for example. I would assume the GUI to the editor would list the names it knew about for the user, or prompt the user if the user wanted to add a new margin name. – jxh Aug 13 '12 at 21:20
0

You can use "relative pointer" which points the distance from the address of struct to point a special element. For example:

SetMargin(FIRST,5);

Which FIRST is from an enum value and is 0.

SetMargin(SECOND,100);

SECOND is 4 since int is 4 bytes in my system

implementation:

void SetMargin(enum margin_elements,int a)
{
    int *relative=struct_pointer+(int*)margin_elements;
    *relative_pointer=a;
     return;
 }
huseyin tugrul buyukisik
  • 11,469
  • 4
  • 45
  • 97
0

If you can make margin public, you can get rid of get,set methods:

class myClass {
    public:
        struct Margin {
            int bottom;
            int left;
            int right;
            int top;
        };
        Margin margin;
};

myClass mc;

mc.margin.bottom = mc.margin.left;
perreal
  • 94,503
  • 21
  • 155
  • 181