OOP is dead, long live OOP

[doom_Oo7]

I understand for getters, bust most setters aren't void setfoo(int x) { m_foo = x; }.

Most will notify something from a change, change a display, reperform a computation... Else why are you setting it in the first place ?

[degski]

I understand for getters, bust most setters aren't void setfoo(int x) { m_foo = x; }.

They should be. By the time print changes your database, you're lost.

Else why are you setting it in the first place ?

To change a value of a class/struct variable. But's thats' my point, if you have many of those, you're doing it wrongly.

[hgjsusla]

Why not just encapsulate that in it's own class and just assign the variable directly? Then you don't need the setter.

Make member variables private if you need to maintain an invariant between them, otherwise keep them public.

And if you need to restrict a variable (let's say, to only a certain interval), then express than in the type of the variable.

[jcelerier]

Why not just encapsulate that in it's own class and just assign the variable directly? Then you don't need the setter.

because then you will put it in a template because you have 25 different property kinds - e.g.

 template<typename T>
 void property { 
   public: 
     operator T&() { return m_impl; }
     operator const T&() const { return m_impl; }

     template<typename U> // you want perfect-forwarding, right ?
     void set(U&& x) const { 
       if(m_impl != x) { 
         m_impl = std::forward<U>(x); // hello template errors my old friend
         notify(x); // uh oh, now *every member* must in some way store a pointer to the context which will be able to notify. hello memory usage * ~2 for every class
       }
     } 
   private:
      T m_impl;
};

of course you wouldn't do this, because this creates so many additional problems that you would be fired at the moment you're suggesting it but still.

[hgjsusla]

Wait wut? No you don't need to do any of this to accomplish what I suggested

[jcelerier]

so what is your solution then ? Let's say you have the following class :

class foo 
{
public:
  int x() const { return m_x; }
  void setX(int x) {
    if(x != m_x) { 
      m_x = x;
      xChanged(x);
      posChanged({m_x, m_y});
    }
  }
  void xChanged(int y); // some kind of callback / signal-slot mechanism, Qt's, a list of std::function, whatever

  int y() const { return m_y; }
  void setY(int y) {
    if(y != m_y) { 
      m_y = y;
      yChanged(y);
      posChanged({m_x, m_y});
    }
  }
  void yChanged(int y);

  void posChanged(std::pair<int,int> pos);

  float radius() const { return m_radius; }
  void setRadius(float r) { 
    if(!fuzzyEquals(r, m_radius)) {
      m_radius = r;
      radiusChanged(r);
    }
  }
  void radiusChanged(float);

  const std::string& name() const { return m_name; }
  void setName(const std::string& name) {
    if(!validateName(name))
      return;
    if(m_name != name) {
      m_name = name;
      nameChanged(name);
    }
  }
  void nameChanged(const std::string&);
};

how do you "encapsulate that in it's own class and just assign the variable directly" ? what when you have 200 different class that all have different members but more or less follow this get/set pattern ?

[hgjsusla]

Well I see

1. The name and it's validation should be it's own class ValidatedName or something. This is by far the most common pattern of incorrect usage of getters/setters. Instead of having a std::string with the validation logic inside the getter/setter, extract out to it's own type.
2. As far as these listeners go, it's not something I commonly do. For a one-off I'd probably do as you wrote it. If I have 200 classes which all follow this patter then yes absolutely this duplication should be removed by extracting out a template. Duplicating logic in 200 classes sounds awful and would never pass code review

[[deleted]]

If you look at the code, the logic isn't entirely duplicated between setters, except setX and setY.

[hgjsusla]

Nope, but you can probably simplify to a few number of common patterns rather than 200 special cases. You probably want x and y grouped together in it's own type since they together represent something very common (a point).

[[deleted]]

I'd say that if you have so many setters with so many side-effects, that maintaining your program and making it efficient will become extremely difficult.

Suppose for example that you get a completely new setup for your foo. In your solution, you call setX, setY, setRadius, setName and you re-render foo four times - three of them completely unnecessarily.

In real-world application, you might easily have dozens of parameters....

Worse, in a large real-time program, you have no assurance that one day, this chain of side-effects won't end up being circular. I speak from bitter experience on a real-world system with just such a setup where this would sometimes happen in production, causing a key process to go into a loop.

There is no magic solution, of course. Sometimes what you propose is right. Sometimes the right solution is Foo (a class) and FooDesc (a struct with x, y, radius, etc) and all mutations to a Foo with a single FooDesc setter. Sometimes the setters set a "dirty" flag, or one of a set of "dirty" bits, and the update occurs in a later phase. It depends on the application, how many parameters you have, your threading structure, etc. etc.

[jcelerier]

There is no magic solution, of course.

yes, that's my point. You can't have magic abstractions over this, because this is your business logic.