Reputation: 3044

How to merge two functions with same conditions?

I quickly wrote the below class for this question.

I'm looking for a way to merge addFruit() with removeFruit() to reduce the code.

They both use identical conditions but just different function call at the end.

My Code :

#include <iostream>
#include <string>
#include <vector>

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        if (str == "apples")
            addToVec(apples, count);
        else if (str == "oranges")
            addToVec(oranges, count);
        else if (str == "lemons")
            addToVec(lemons, count);
        else if (str == "melons")
            addToVec(melons, count);
        else if (str == "bananas")
            addToVec(bananas, count);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }
    void removeFruit(const std::string &str)
    {
        if (str == "apples")
            removeFromVec(apples);
        else if (str == "oranges")
            removeFromVec(oranges);
        else if (str == "lemons")
            removeFromVec(lemons);
        else if (str == "melons")
            removeFromVec(melons);
        else if (str == "bananas")
            removeFromVec(bananas);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }
private:
    void addToVec(std::vector<int> &vec, int count)
    {
        vec.push_back(count);
    }
    void removeFromVec(std::vector<int> &vec)
    {
        vec.pop_back();
    }
    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
};

Any clever way to nicely merge the two functions so I can reduce the code?

Upvotes: 6

Answers (9)

Leubh

Reputation: 11

I would do this:

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        doToFruit(str, [&](std::vector<int> &vec){ addToVec(vec, count); });
    }
    void removeFruit(const std::string &str)
    {
        doToFruit(str, [&](std::vector<int> &vec){ removeFromVec(vec); });
    }
private:
    template<typename Callable>
    void doToFruit(const std::string &str, const Callable &func)
    {
        std::pair<const char*, std::vector<int>&> fruits[] = {
            {"apple", apples}, {"oranges", oranges}, {"lemons", lemons},
            {"melons", melons}, {"bananas", bananas}
        };
        for (auto i : fruits)
            if (str == i.first)
                return func(i.second);
        std::cout << "Unknown Fruit : " << str << '\n';
    }
    void addToVec(std::vector<int> &vec, int count)
    {
        vec.push_back(count);
    }
    void removeFromVec(std::vector<int> &vec)
    {
        vec.pop_back();
    }
    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
};

You can use pointer to members if you want better performance:

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        doToFruit(str, [&](std::vector<int> &vec){ addToVec(vec, count); });
    }
    void removeFruit(const std::string &str)
    {
        doToFruit(str, [&](std::vector<int> &vec){ removeFromVec(vec); });
    }
private:
    template<typename Callable>
    void doToFruit(const std::string &str, const Callable &func)
    {
        auto iFound = fruits.find(str);
        if (iFound != fruits.end())
            return func(this->*(iFound->second));
        std::cout << "Unknown Fruit : " << str << '\n';
    }
    void addToVec(std::vector<int> &vec, int count)
    {
        vec.push_back(count);
    }
    void removeFromVec(std::vector<int> &vec)
    {
        vec.pop_back();
    }
    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
    static std::unordered_map<std::string, std::vector<int> MyClass::*> fruits;
};

std::unordered_map<std::string, std::vector<int> MyClass::*> MyClass::fruits = {
    {"apple", &MyClass::apples}, {"oranges", &MyClass::oranges},
    {"lemons", &MyClass::lemons}, {"melons", &MyClass::melons},
    {"bananas", &MyClass::bananas}
};

Upvotes: 1

Manthan Tilva

Reputation: 3277

What about below solution. That will also allow you add/remove known fruits easily by adding/removing line in constructor.

#include <iostream>
#include <string>
#include <vector>
#include <map>

class MyClass
{
public:
    MyClass()
    {
        allowedFruits["apples"] = {};
        allowedFruits["oranges"] = {};
        allowedFruits["lemons"] = {};
        allowedFruits["melons"] = {};
        allowedFruits["bananas"] = {};
    }
    void addFruit(const std::string &str, int count)
    {
        auto it = allowedFruits.find(str);
        if(it != MyClass::allowedFruits.end()){
            it->second.push_back(count);
        }
        else {
            std::cout << "Unknown Fruit : " << str << '\n';
        }
    }
    void removeFruit(const std::string &str)
    {
        auto it = allowedFruits.find(str);
        if(it != allowedFruits.end()){
            // my be some check here
            it->second.pop_back();
        }
        else {
            std::cout << "Unknown Fruit : " << str << '\n';
        }
    }
private:
    std::map<std::string,std::vector<int>> allowedFruits;
};

Upvotes: 2

Rizwan

Reputation: 3666

A code that selects the vector to be used and then perform the action can be used :

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        auto vec = selectVector(str);
        if(vec != nullptr)
            addToVec(*vec, count);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }
    void removeFruit(const std::string &str)
    {
        auto vec = selectVector(str);
        if(vec != nullptr)
            removeFromVec(*vec);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }
private:

    std::vector<int> *selectVector(const std::string &str)
    {
        if (str == "apples")
            return &apples;
        else if (str == "oranges")
            return &oranges;
        else if (str == "lemons")
            return &lemons;
        else if (str == "melons")
            return &melons;
        else if (str == "bananas")
            return &bananas;
        else
            return nullptr;
    }

    void addToVec(std::vector<int> &vec, int count)
    {
        vec.push_back(count);
    }
    void removeFromVec(std::vector<int> &vec)
    {
        vec.pop_back();
    }
    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
};

Upvotes: 2

dkg

Reputation: 1775

I would go a little functionnal by passing the function to apply.

#include <iostream>
#include <string>
#include <vector>
#include <functional>

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        searchAndApplyHelper(str, std::bind(&MyClass::addToVec, *this, std::placeholders::_1, count));
    }
    void removeFruit(const std::string &str)
    {
        searchAndApplyHelper(str, std::bind(&MyClass::removeFromVec, *this, std::placeholders::_1));
    }

private:

    template <class Func>
    void searchAndApplyHelper(const std::string str, Func f)
    {
        if (str == "apples")
            f(apples);
        else if (str == "oranges")
            f(oranges);
        else if (str == "lemons")
            f(lemons);
        else if (str == "melons")
            f(melons);
        else if (str == "bananas")
            f(bananas);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }

    void addToVec(std::vector<int> &vec, int count)
    {
        vec.push_back(count);
    }
    void removeFromVec(std::vector<int> &vec)
    {
        vec.pop_back();
    }
    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
};

I did it with a template but you could use std::function as well.

Upvotes: 1

Nick is tired

Reputation: 7055

Using C++17 you could use optional arguments:

void addRemoveFruit(const std::string &str, std::optional<int> count = std::nullopt)
{
    if (str == "apples")
        addRemoveVec(apples, count);
    else if (str == "oranges")
        addRemoveVec(oranges, count);
    else if (str == "lemons")
        addRemoveVec(lemons, count);
    else if (str == "melons")
        addRemoveVec(melons, count);
    else if (str == "bananas")
        addRemoveVec(bananas, count);
    else
        std::cout << "Unknown Fruit : " << str << '\n';
}

and:

void addRemoveVec(std::vector<int> &vec, std::optional<int> count = std::nullopt)
{
    if(count.has_value()) {
        vec.push_back(count.value());
    } else {
        vec.pop_back();
    }
}

This way existing calls to addFruit/removeFruit only need to be changed to addRemoveFruit without having to change the parameters passed.

Upvotes: 0

Max Vollmer

Reputation: 8598

To stick with your pattern of multiple distinct vectors, I'd recommend using an internal enum that decides the method, while the choice of the vector can be done in one place:

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        addOrRemoveFruit(str, count, Method::ADD);
    }
    void removeFruit(const std::string &str)
    {
        addOrRemoveFruit(str, 0, Method::REMOVE);
    }

private:
    enum class Method
    {
        ADD,
        REMOVE
    };

    void addOrRemoveFruit(const std::string &str, int count, Method method)
    {
        if (str == "apples")
            addOrRemoveFruitImpl(apples, count, method);
        else if (str == "oranges")
            addOrRemoveFruitImpl(oranges, count, method);
        else if (str == "lemons")
            addOrRemoveFruitImpl(lemons, count, method);
        else if (str == "melons")
            addOrRemoveFruitImpl(melons, count, method);
        else if (str == "bananas")
            addOrRemoveFruitImpl(bananas, count, method);
        else
            std::cout << "Unknown Fruit : " << str << '\n';
    }

    void addOrRemoveFruitImpl(std::vector<int> &vec, int count, Method method)
    {
        if (method == Method::ADD)
             vec.push_back(count);
        else
             vec.pop_back();
    }

    std::vector<int> apples;
    std::vector<int> oranges;
    std::vector<int> lemons;
    std::vector<int> melons;
    std::vector<int> bananas;
};

However even better would be to use a map:

class MyClass
{
public:
    void addFruit(const std::string &str, int count)
    {
        fruits[str].push_back(count);
    }
    void removeFruit(const std::string &str)
    {
        if (fruits.count(str) > 0)
            fruits[str].pop_back();
    }

private:
    std::unordered_map<std::string, std::vector<int>> fruits;
};

Upvotes: -1

πάντα ῥεῖ

Reputation: 1

The easiest solution might be to use a std::map for those vectors:

std::map<std::string,std::vector<int>> fruitVecs;

The key values of the map would be "apples", "oranges", "bananas" etc.

Thus you can easily access the corresponding vector for any operation through the map.

Upvotes: 3

463035818_is_not_an_ai

Reputation: 122585

Without changing the interface you could do it like this:

std::vector<int>& pickVector(std::string str) {
    // put all the switch here and return a reference to the correct vector
}

void addFruit(const std::string &str, int count)
{
   addToVec(pickVector(str),count);
}

Upvotes: 1

Korni

Reputation: 464

Make an additional function e.g. determineTargetVector(const std::string &str) which returns the corresponding vector, where you want to insert/remove an element, so you have no redundant conditions. Also its nice to have only a single reponsibility for each function.

For example:

std::vector<int> *determineTargetVector(const std::string &str)
{
    if (str == "apples")
        return &apples;
    else if (str == "oranges")
        return &oranges;
    else if (str == "lemons")
        .
        .
        .
    else
        //something invalid, to check for in superior function
        return nullptr;
}

Upvotes: 6

How to merge two functions with same conditions?

Answers (9)

Related Questions