0

I have about 20 private bools within a class in C++. I would like for these to be publicly accessible using a (public) function.

Is it possible to pass the name of a (private) variable as an argument to such a function? For example,

void setTrue(std::string varName)
{
    someFunctionToConvertStringToVariable = true;
}

Alternatively, I think that

void setTrue(std::string varName)
{
    if (varName == "boolA")
    {
        boolA = true;
    }
    else if (varName == "boolB")
    {
        boolB = true;
    }
}

would work, and could use a switch(varName) to reduce the number of LOC needed.

Another option would presumably be to just make all of the booleans public, and then access them using myClass.boolA = true; from the calling program - I'm not sure this is the best idea, but it's certainly simpler, and so that's an argument in its favour.

Is there a generally accepted/best way to do this type of thing? Have I just set up the problem badly and is there a much smarter way to go about this? Perhaps an enum of varnames would allow passed variables to be checked, but I don't think that would necessarily make it easier to set the boolean.

chrisb2244
  • 2,940
  • 22
  • 44
  • 1
    Instead of 20 private variables you can have an array of 20, and pass index to change in public member function. – P0W Jul 29 '14 at 05:22
  • 2
    Having 20 private `bool`s in a single class is a massive design smell - it means your class is badly designed and the way you are trying to model whatever problem you're solving needs to be rethought. – Yuushi Jul 29 '14 at 05:22
  • Probably true, but they're for some 10k lines of code, and the bools govern in a `if(boolA) cout<< "silly description of whats happening now";` kind of way. The length of a log file varies on multiple orders of magnitude depending on which bools are set, and so if I only want to find out what's wrong with one part, setting most to false is helpful. – chrisb2244 Jul 29 '14 at 05:25
  • Making them changeable from a calling program is more helpful for other users though, who don't want to have to recompile a library over and over, or (more importantly) who want to change the value mid program, running in a loop of delta time periods (ie, program breaks at t=30s, don't spam log for first 29s) – chrisb2244 Jul 29 '14 at 05:26

4 Answers4

3

You can use a std::map<std::string, bool> to store the bool values. Then,

void setTrue(std::string varName)
{
    // Add some checks to make sure that varName is valid.

    varNames[varName] = true;
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Going to try this - suspect it will be the answer I need though – chrisb2244 Jul 29 '14 at 05:28
  • 1
    The check if varname is valid is by checking the return value of `map::find (key)` instead of using the random access... if it is equal to `map::end` it was not valid. – Theolodis Jul 29 '14 at 05:32
  • Right answer, but please change the `std::string` to a const ref as well as using `find`. – Keith Jul 29 '14 at 05:34
  • As others pointed too, `std::map::operator []` shouldn't be used over here, since if `varName` is not present it will add it into `varNames` and that's not the point, use `std::map::find` – P0W Jul 29 '14 at 05:46
  • This is certainly the answer I asked for, so marked it accepted. It may be having looked into this, that my question was not desperately well posed, but this gives me something to look into/work on. Thanks – chrisb2244 Jul 29 '14 at 05:47
  • Note that you could use a map of `bool*` if you want to keep the original member list for convenience inside the class. – Keith Jul 29 '14 at 05:51
  • @chrisb2244 the objections are that you could create new map entries by passing in strings you weren't expecting; and if you don't want that behaviour then the solution is to create all the named map entries in your constructor, and use `map::find` instead of `map::operator[]`. – M.M Jul 29 '14 at 05:54
  • @Keith - Thanks, this is helpful - I was working on trying to get pointers in the right place to do that, but now it was simple with your tip. @MattMcNabb - Also thanks, will place that inside my function. It's now looking pretty messy, but at least the implementation is encapsulated away from the user - `*(object.myMap["myName"]) = true/false;` and so on. Will make it `...myMap.find("myName")` – chrisb2244 Jul 29 '14 at 06:39
2

If the list of these variables isn't going to change, I can propose you to use enumeration and overriden subscript operator:

enum MyBools
{
    First = 0,      
    BoolA = First,       
    BoolB,       
    //...
    Bool20,     
    Last = Bool20,
}    

class MyIndexedBools
{
private:
    bool m_Bools[MyBools::Last + 1];

public:
    bool& operator[] (MyBools index);
};

bool& operator[] (MyBools index);
{
    if (index < First || index > Last)
         throw "error";

    return m_Bools[index];
}

It is not extensible in runtime but will give you better compile-time safety than maps.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
Eugene Podskal
  • 10,270
  • 5
  • 31
  • 53
  • An enum is one way indeed. In C++11, you could precise you want the enum to have `size_t` as the backing storage so that no conversion is needed when converting to an index... and also because with an unsigned storage `First` becomes unnecessary. The next step would be to use `std::array`. – Matthieu M. Jul 29 '14 at 08:29
0

Your second solution is the way to go here, however I think that the switch case does not reduce the LOC, beside the fact that it won't work with an `std::string!

void setTrue(const std::string& varName)
{
    if (varName == "boolA")
    {
        boolA = true;
    }
    else if (varName == "boolB")
    {
        boolB = true;
    }
}

VS:

void setTrue(const std::string& varName)
{
    switch(str2int (varName))
    {
    case str2int ("boolA"):
        boolA = true;
        break;
    case str2int("boolB"):
        boolB = true;
        break;
    default: 
        assert (false);
    }
}

And keep in mind that if you do not want to modify the passed parameter it is good practice to pass it as const! In that case I'd personally pass it as const reference.

Community
  • 1
  • 1
Theolodis
  • 4,977
  • 3
  • 34
  • 53
  • You missed copying `str2int` implementation from above link – P0W Jul 29 '14 at 05:27
  • I am not here to duplicate code, it was just to give an idea of the LOC.... I did link the original question, so feel free to take it from there ;) – Theolodis Jul 29 '14 at 05:29
0

Twenty bool is a lot, indeed.

Instead, I would advise using a bitset indexed with constants (using enum for example). Assuming C++11:

class SomeClass {
public:
    enum class Flag: size_t {
        Flag1,
        Flag2,
        Flag3,
        ...,
        Flag20
    };

    bool get_flag(Flag f) const { return _flags.test(size_t(f)); }

    void set_flag(Flag f, bool value) { _flags.set(size_t(f), value); }

private:
    std::bitset<size_t(Flag20) + 1> _flags;
}; // class SomeClass

Note: all out of range accesses are carried out by bitset, handy isn't it ?

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722