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?
2 Answers 2
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;
-
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– matt2016年05月13日 18:47:49 +00:00Commented May 13, 2016 at 18:47
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;
}
-
2\$\begingroup\$ ...or with linq
return objs.Aggregate(true, (current, next) => current & obj.Method())
\$\endgroup\$t3chb0t– t3chb0t2019年06月14日 16:50:11 +00:00Commented Jun 14, 2019 at 16:50