6
\$\begingroup\$

I was challenging myself to make a way to easily perform actions on a list of objects. The idea is to prevent the copying of the list. I came up with the following, what I think is quite elegant. But I really dislike the casting of the pointers here. I would like to use smart pointers as well, but refrain from using RTTI.

On the to-do list, after the first action, it should return a different class, so you can only do more actions. So no more filtering possible after the first action.

What do you think? Is something like this a good idea to use? (Of course, this is a dumbed-down version. The real classes and conditions are more complicated.)

#include <iostream>
#include <memory>
#include <functional>
#include <vector>

struct Person
{
    std::string Name;
    int Age;

};

class PersonActions
{
    std::function<void(Person*)> actions = [](Person* device){};  // No-op by default
public:
    void Append(std::function<void(Person*)> action)
    {
        auto oldAction = actions;
        actions = [oldAction, action](Person *device){
            oldAction(device);
            action(device);
        };
    }

    void Execute(Person* device)
    {
        actions(device);
    }
};


template<typename T>
class PersonActionsBuilder
{
protected:
    PersonActions actions;

public:

    T& ExecuteCustom(std::function<void(Person*)> action)
    {
        actions.Append(action);
        return *((T*)this);
    }

    T& AppendName(std::string value){
        actions.Append([value](Person *device) {
            device->Name.append(value);
        });
        return *((T*)this);
    }

    T& Print()
    {
        actions.Append([](Person *device) {
            std::cout << " - Name: " << device->Name <<  std::endl;
            std::cout << " - Age:  " << device->Age <<  std::endl;
        });
        return *((T*)this);
    }
};


class PersonQuery 
{
    std::function<bool(Person*)> predicate = [](Person* device){ return true; };  // Always true by default

public:
    void Append(std::function<bool(Person*)> newPredicate)
    {
        auto oldPredicate = this->predicate;
        this->predicate = [oldPredicate, newPredicate](Person *device){
            return oldPredicate(device) && newPredicate(device);
        };
    }

    bool Match(Person* device)
    {
        return predicate(device);
    }
};


template<typename T>
class PersonQueryBuilder
{
protected:
    PersonQuery query;
public:

    T& WhereCustom(std::function<bool(Person*)> predicate)
    {
        query.Append(predicate);
        return *((T*)this);
    }

    T& WhereAge(int value, std::function<bool(int, int)> comparator = std::equal_to<bool>())
    {
        query.Append([value, comparator](Person *device) {
            return comparator(device->Age, value);
        });
        return *((T*)this);
    }
};


class PersonExecutor
{
public:

    void Execute(PersonActions& actions, PersonQuery& query, std::vector<Person*>& devices)
    {
        for(auto device : devices)
        {
            if(query.Match(device))
            {
                actions.Execute(device);
            }
        }
    }
};



class PersonExecutableQuery : public PersonQueryBuilder<PersonExecutableQuery>, public PersonActionsBuilder<PersonExecutableQuery>
{
    std::vector<Person*>& devices;
public:

    PersonExecutableQuery(std::vector<Person*>& devices) : devices(devices)
    {
    }

    void Execute()
    {
        PersonExecutor executor;
        executor.Execute(actions, query, devices);
    }
};

int main() {
    std::vector<Person*> persons;
    persons.push_back(new Person{"John Doe", 30});
    persons.push_back(new Person{"Jane Smith", 25});
    persons.push_back(new Person{"Alice Johnson", 22});
    persons.push_back(new Person{"Bob Brown", 45});
    persons.push_back(new Person{"Charlie Davis", 28});


    
    
    PersonExecutableQuery(persons)
        .WhereAge(30, std::greater_equal<int>())
        .AppendName(" yep")
        .Execute();
    
    PersonExecutableQuery(persons)
        .WhereAge(30, std::less<int>())
        .AppendName(" nope")
        .Execute();
        
    PersonExecutableQuery(persons)
        .WhereCustom([](Person* person){
            return person->Name.ends_with("yep");
        })
        .Print()
        .Execute();
        

}

\$\endgroup\$

2 Answers 2

11
\$\begingroup\$

Imperative programming to the rescue

C++ is a multi-paradigm language; it provides multiple ways to solve problems, and some ways are simpler than others. You have gone the object-oriented route, and Toby Speight showed how to do it in a more functional programming style. However, there is also the good old imperative programming way you could use. It looks like this:

int main() {
    std::vector<Person> persons{
        {"John Doe",      30},
        {"Jane Smith",    25},
        {"Alice Johnson", 22},
        {"Bob Brown",     45},
        {"Charlie Davis", 28},
    };
    
    for (auto& person: persons)
        if (person.Age >= 30)
            person.Name += " yep";
    
    for (auto& person: persons)
        if (person.Age < 30)
            person.Name += " nope";
        
    for (auto& person: persons)
        if (person.Name.ends_width("yep"))
            std::cout << " - Name: " << person.Name << "\n"
                      << " - Age:  " << person.Age <<  "\n";
}

Note how you didn't need to create any classes, and you didn't need to learn any of the standard library ranges/views/algorithms, just for and if. It's also very flexible.

\$\endgroup\$
4
  • 4
    \$\begingroup\$ Simple and readable and understandable. Toby's and OP's versions were scary. \$\endgroup\$
    – Harith
    Commented May 22 at 19:50
  • \$\begingroup\$ I get where you are coming from, the thing is that on the real objects the conditions are more complex. So in this example, I agree that it's more readable. But when you start to get multiple nested if statements checking all kinds of things. It maybe more readable to go to a query style. Although, I guess this is also preference. I do agree that your solution is readable by everyone! \$\endgroup\$ Commented May 23 at 6:26
  • 1
    \$\begingroup\$ Too late for the edit. I guess when things get more complicated, you can always make dedicated functions. Especially if they are used on multiple occasions. For example, bool CheckIfPersonIs.... \$\endgroup\$ Commented May 23 at 6:34
  • \$\begingroup\$ Since I'm not the only person working on this, I feel like this should be the accepted answer. This is more understandable for others. \$\endgroup\$ Commented May 23 at 8:49
7
\$\begingroup\$

The functionality is similar to standard views, but with an unconventional syntax, using function calls and the Builder pattern, rather than the | operator. C++ programmers are more used to something like

auto under_30s = persons | std::views::filter(compare_proj(&Person::Age, std::less<>{}, 30))
std::ranges::for_each(under_30s, append(&Person::Name, " nope");

or even plainer:

    std::ranges::copy(persons
                      | std::views::filter([](auto& p){ return p.Age < 30; })
                      | std::views::transform([](auto& p){ p.Name += " yep"; return p;}),
                      std::ostream_iterator<Person>{std::cout, "\n"});

No need to write and maintain complex machinery yourself when the standard library is so supportive!


These casts are Undefined Behaviour:

    return *((T*)this);

The …Builder<T> classes are not derived from T, so there's no conversion between their pointers.


Why are we flushing output with std::endl instead of '\n' here?

        std::cout << " - Name: " << device->Name <<  std::endl;
        std::cout << " - Age:  " << device->Age <<  std::endl;

And why can we print only to standard output? We ought to be passing the destination stream as a parameter.


We leak memory in main() because we have a vector of raw, owning pointers. Avoid doing that; here, it's simple to store Person objects: std::vector<Person> persons. If we were to extend this and make Person a polymorphic base class, then might consider using std::vector<std::unique_ptr<Person>>. Never own a resource using a raw pointer!


On the to-do list, … no more filtering possible after the first action.

Why? It might be reasonable to act on a set of items and perform further actions on a subset of them.

\$\endgroup\$
5
  • \$\begingroup\$ Ah, I haven't been programming in C++ for long, so a lot of features are still unknown to me. In this case, std::views is something I haven't come across. I agree that a more standard approach would be better. I do have one question, can the transform be done without copying the vector? I am on an embedded device, so I really need to know what's going on when I do these things. Dynamic memory is all fun and games, until you have limited resources. (Oh, of course I would use smart pointers and the print is just part of the example.) \$\endgroup\$ Commented May 23 at 6:29
  • 1
    \$\begingroup\$ Standard views don't copy collections (but the flip side is that accessing elements of views may re-calculate them on each access, and the programmer is responsible for ensuring the elements' storage outlives their access through the view). Whenever you see view in a name (such as std::string_view), that tells you that it accesses storage that's owned by something else. \$\endgroup\$ Commented May 23 at 8:05
  • 1
    \$\begingroup\$ BTW, the std::ranges::copy() I wrote is copying the vector's content to the standard output stream - i.e. printing. It doesn't involve making an additional copy in memory. \$\endgroup\$ Commented May 23 at 8:06
  • \$\begingroup\$ Thanks for the clarification \$\endgroup\$ Commented May 23 at 8:40
  • \$\begingroup\$ Although I didn't set this as the accepted answer, I am thankful for learning something new. \$\endgroup\$ Commented May 23 at 8:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.