6
\$\begingroup\$

I have the case of repeated very similar logic that is used to check if a boolean value was changed from the initial value and prevent it from changing back, or to summarize multiple boolean return values like:

bool SomeMethod()
{
    bool endResult = true;
    foreach(var obj in objs)
    {
        bool result = obj.Method();
        if(!result)
            endResult = false;
    }
    return endResult;
}

I reworked the logic into a class where a boolean can only be changed from the initial value, so that the boolean logic is all in one place. The IsChanged property is for LINQ queries.

public class OneUseBool
{
    private readonly bool _initValue;

    public OneUseBool(bool init)
    {
        _initValue = init;
        _currentValue = init;
        _isChanged = false;
    }

    private bool _currentValue;
    public bool CurrentValue
    {
        get { return _currentValue; }
        set
        {
            if (_initValue != value)
            {
                _currentValue = value;
                IsChanged = true;
            }
        }
    }

    private bool _isChanged;
    public bool IsChanged
    {
        get { return _isChanged; }
        private set
        {
            if(!_isChanged)
                _isChanged = value;
        }
    }
}

So the original code gets changed to:

bool SomeMethod()
{
    OneUseBool endResult = new OneUseBool(true);
    foreach(var obj in objs)
    {
        endResult.CurrentValue = obj.Method();
    }
    return endResult.CurrentValue;
}

Is this a good way to do this sort of logic with bools, or am I just overcomplicating a simple problem?

\$\endgroup\$
6
\$\begingroup\$

If you can only change a bool once, and the entire purpose of a loop is to set the bool, then you should just exit the loop after the bool gets changed. There's no point in looping more than you need to, and you also don't have to worry about the bool getting switched back. So you can write your example like this:

bool SomeMethod()
{
    foreach(var obj in objs)
    {
        if(!obj.Method())
        {
            return false;
        }
    }
    return true;
}

You can use Enumerable.All() for this logic:

bool SomeMethod()
{
    return objs.All(x => x.Method());
}

An alternative to if(!x) y = x; in your situation is y = y && x;

\$\endgroup\$
  • 1
    \$\begingroup\$ That should handle the majority of cases, enough that I don't think it's worth keeping the new class. Thanks! \$\endgroup\$ – matt May 13 '16 at 18:47
2
\$\begingroup\$

If you don't need to short-circuit the loop once a intermediate result is false, you can write the following change to your loop. All items in the loop are invoked, and the end result is true only when all invocations yield true.

bool SomeMethod()
{
    bool endResult = true;
    foreach(var obj in objs)
    {
        endResult &= obj.Method();
        //bool result = obj.Method();
        //if(!result)
        //    endResult = false;
    }
    return endResult;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ ...or with linq return objs.Aggregate(true, (current, next) => current & obj.Method()) \$\endgroup\$ – t3chb0t Jun 14 at 16:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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