7
\$\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?

dfhwze
14.1k3 gold badges40 silver badges101 bronze badges
asked May 13, 2016 at 17:33
\$\endgroup\$

2 Answers 2

7
\$\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;

answered May 13, 2016 at 18:27
\$\endgroup\$
1
  • 1
    \$\begingroup\$ That should handle the majority of cases, enough that I don't think it's worth keeping the new class. Thanks! \$\endgroup\$ Commented May 13, 2016 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;
}
answered Jun 14, 2019 at 16:04
\$\endgroup\$
1
  • 2
    \$\begingroup\$ ...or with linq return objs.Aggregate(true, (current, next) => current & obj.Method()) \$\endgroup\$ Commented Jun 14, 2019 at 16:50

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.