HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Variadic templates and pointers to member functions to achieve a named-parameters interface in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
achievenamedinterfacemembertemplatesvariadicpointersandfunctionsparameters

Problem

I studied a bit and packed all the suggestions that I received here: Fluent interface and polymorphism for building a scene with shapes and I came up with this:

#include
#include
#include
#include

using namespace std;

struct Figure {
string _name;
virtual double area() const=0;
virtual ~Figure() {}
};

struct Circle: Figure {
double _radius;
Circle * radius(double r) { _radius=r; return this;}
Circle * name(string str) { _name=str; return this;}
double area() const override {return M_PI_radius_radius;}
~Circle() {}
};

struct Square: Figure {
double _side;
Square * side(double s) { _side=s; return this;}
Square * name(string str) { _name=str; return this;}
double area() const override {return _side*_side;}
~Square() {}
};

struct Scene {
vector v;
~Scene() { for (auto & f : v) delete f; }
double total_area() const {
double total=0;
for (auto f : v) total += f->area();
return total;
}

//here we go, this is the standard function with recursion
template
void apply(T t, T(T::*func)(S), const S & par, const Args &... args) {
(t->*func)(par);
apply(t, args...);
}

//this terminates the recursion
template
void apply(T t, T(T::*func)(S), const S & par) {
(t->*func)(par);
}

//this is for the empty args call
template
void apply(T *t) {
cerr
Scene& add( const Args &... args ) {
T * t = new T();
apply(t, args...);
v.emplace_back(t);
return *this;
}
};

int main() {
Scene scene;
scene.add(&Circle::name,string("c1"),&Circle::radius,1.)
.add(&Square::side,10.,&Square::name,string("s1"))
.add();
cout

The principle looks very nice to me, don't you agree? However the interface is quite heavy. My main concern is about the need to put the name of the object (
&Circle:: or &Square::`) in front of every method in the interface: definitely too rambling, but I have not been able to remove it.

Looking at it for a second time I am thinking that maybe ins

Solution

You said that you packed all the suggestions from the previous question, but there are still some pieces of advice that you did not integrate into your code. Here is how you can still improve your it:

math.h legacy

In your code, you are using the constant M_PI. While it will probably work on most of the compilers I know, this macro isn't defined anywhere in any C or C++ standard. The math.h/cmath constants are only a common POSIX extension. You should define your own math constant somewhere in order to avoid portability problems. Since it was the only feature from ` you used, you can remove the include.

Perfect forwarding

There are many places where you should use
std::forward to perfectly forward your arguments from a function to another. For example, you could turn this piece of code:

template
void apply(T *t, const S & setter, const Args &... args) {
  t->set(setter);
  apply(t, args...);
}


into this one:

template
void apply(T *t, const S & setter, Args &&... args) {
  t->set(setter);
  apply(t, std::forward(args)...);
}


std::unique_ptr

Avoid raw pointers. Instead, use a
std::unique_ptr which will manage the memory alone. It's by far safer. Since the modified portion of the code is quite huge for this modification, I won't include it in this section. However, you can find the modified code at the end of my post :)

Constructor inheritance

Do not bother creating specialized constructors to forward your results to
Setter from its derived classes. You can use constructor inheritance:

struct Name: Setter { using Setter::Setter; };
struct Side: Setter { using Setter::Setter; };
struct Radius: Setter { using Setter::Setter; };


Place of method
set

Since
_name belongs to Figure, you better define void set(const Name& n) in Figure. That way, its derived classes won't have to duplicate the code, and Figure will handle its member variables by itself. However, remember to add using Figure::set; in the derived classes, otherwise, the name will be hidden by the overloads.

Choice of the collection

Using a
std::vector to store collections of elements is often the best choice. However, if you plan to remove elements from the middle of your collection at some time, you could consider using std::list instead. Anyway, I don't know what you plan to do with this code, so I did not change the std::vector to an std::list in my revised version of your code (update: actually, be careful anyway if you decide to use an std::list).

Encapsulation

From what I see, all your variables beginning with an underscore ought to be
private members of the classes. However, you may have made them public` in order to simplify your code to post it here, I don't know. I did not modidy that in my revised version of the code in order to keep the code simple.

Here is the revised version of your code. I applied all the aforementioned modifications, appart from the ones which I explicitely stated are not:

#include 
#include 
#include 
#include 

// pi constant
constexpr double pi = 3.141592653589793;

//templated setter for many types
template  
struct Setter {
    T _i;
    Setter(const T & i) : _i(i) {}
    operator T() const {return _i;}
};

//setter for each parameter
struct Name: Setter { using Setter::Setter; };
struct Side: Setter { using Setter::Setter; };
struct Radius: Setter { using Setter::Setter; };

struct Figure {
    std::string _name;
    virtual double area() const=0;
    virtual ~Figure() {}
    void set(const Name & n) { _name = n; }
};

struct Circle: Figure {
    double _radius;
    double area() const override { return pi*_radius*_radius; }
    using Figure::set;
    void set(const Radius & n) { _radius = n; }
};

struct Square: Figure {
    double _side;
    double area() const override { return _side*_side; }
    using Figure::set;
    void set(const Side & n)  { _side = n; }
};

struct Scene {
    std::vector> v;
    double total_area() const {
        double total{};
        for (const auto& f : v)
            total += f->area();
        return total;
    }

    //here we go, this is the standard function with recursion
    template
    void apply(T& t, const S & setter, Args&&... args) {
        t.set(setter);
        apply(t, std::forward(args)...);
    }

    //this terminates the recursion
    template
    void apply(T& t, const S & setter) {
        t.set(setter);
    }

    //this is for the empty args call
    template
    void apply(T&) {
        std::cerr 
    Scene& add(Args&&... args ) {
        std::unique_ptr t(new T());
        apply(*t, std::forward(args)...);
        v.emplace_back(std::move(t));
        return *this;
    }
};

int main() {
    Scene scene;
    scene.add(Name("c1"), Radius(1.))
         .add(Side(10), Name("s2"))
         .add();
    std::cout << "Total area: " << scene.total_area() << std::endl;
}


And you can find the working version here at Coliru.

Code Snippets

template<typename T, typename S, typename ...Args>
void apply(T *t, const S & setter, const Args &... args) {
  t->set(setter);
  apply(t, args...);
}
template<typename T, typename S, typename ...Args>
void apply(T *t, const S & setter, Args &&... args) {
  t->set(setter);
  apply(t, std::forward<Args>(args)...);
}
struct Name: Setter<string> { using Setter::Setter; };
struct Side: Setter<double> { using Setter::Setter; };
struct Radius: Setter<double> { using Setter::Setter; };
#include <iostream>
#include <memory>
#include <string>
#include <vector>

// pi constant
constexpr double pi = 3.141592653589793;

//templated setter for many types
template <typename T> 
struct Setter {
    T _i;
    Setter(const T & i) : _i(i) {}
    operator T() const {return _i;}
};

//setter for each parameter
struct Name: Setter<std::string> { using Setter::Setter; };
struct Side: Setter<double> { using Setter::Setter; };
struct Radius: Setter<double> { using Setter::Setter; };

struct Figure {
    std::string _name;
    virtual double area() const=0;
    virtual ~Figure() {}
    void set(const Name & n) { _name = n; }
};

struct Circle: Figure {
    double _radius;
    double area() const override { return pi*_radius*_radius; }
    using Figure::set;
    void set(const Radius & n) { _radius = n; }
};

struct Square: Figure {
    double _side;
    double area() const override { return _side*_side; }
    using Figure::set;
    void set(const Side & n)  { _side = n; }
};

struct Scene {
    std::vector<std::unique_ptr<Figure>> v;
    double total_area() const {
        double total{};
        for (const auto& f : v)
            total += f->area();
        return total;
    }

    //here we go, this is the standard function with recursion
    template<typename T, typename S, typename ...Args>
    void apply(T& t, const S & setter, Args&&... args) {
        t.set(setter);
        apply(t, std::forward<Args>(args)...);
    }

    //this terminates the recursion
    template<typename T, typename S>
    void apply(T& t, const S & setter) {
        t.set(setter);
    }

    //this is for the empty args call
    template<typename T>
    void apply(T&) {
        std::cerr << "Warning: no parameters set for an object" << std::endl;
    }

    //here is the interface function
    template<typename T, typename ...Args>
    Scene& add(Args&&... args ) {
        std::unique_ptr<T> t(new T());
        apply(*t, std::forward<Args>(args)...);
        v.emplace_back(std::move(t));
        return *this;
    }
};

int main() {
    Scene scene;
    scene.add<Circle>(Name("c1"), Radius(1.))
         .add<Square>(Side(10), Name("s2"))
         .add<Circle>();
    std::cout << "Total area: " << scene.total_area() << std::endl;
}

Context

StackExchange Code Review Q#42748, answer score: 13

Revisions (0)

No revisions yet.