0

In the following (paraphrased) code, I am checking various conditions, updating them, and scheduling a single notification regardless of the number of changed conditions. If none of the conditions are changed, no notification should go out. In addition, there is one special condition that does not trigger a notification if it changes.

public bool Update() {
    bool shouldNotify = false;

    if (SpecialConditionChanged())
        UpdateSpecialCondition();
    
    if (Condition1Changed()) {
        UpdateCondition1();
        shouldNotify = true;
    }

    if (Condition2Changed()) {
        UpdateCondition2();
        shouldNotify = true;
    }

    ...

    return shouldNotify;
}

This code works, but it is very verbose and repetitive having to set the flag every time. I am asking if there is a more elegant or efficient solution. Something that will return false if either zero conditions or only the special condition have changed, but will return true if any of the remaining conditions have changed.

I've thought about using a bool[] (just as verbose); I've thought about using a counter (just as repetitive); I've thought about adding an extra if that ors all the conditions together to change the boolean once (there are a lot of conditions), but nothing I've thought about is any cleaner than this version.

Am I missing something that could make this look or work better?

Improve this question
7
  • 3
    It works but you do not particularly like the looks - from experience: leave it alone. Let it sit for a while and do something else. The improved version will come to you naturally. - Or never. Which may just be good enough. Commented Oct 15, 2024 at 14:11
  • 1
    @Fildor Would have been cool if I knew that some years ago. Yeap, sometimes you are too close to get a clear view on a problem just to realize you don't even have a problem. Commented Oct 15, 2024 at 14:26
  • Something like this => dotnetfiddle.net/VtGoIt - I wouldn't really consider as an improvement. Commented Oct 15, 2024 at 14:27
  • 2
    I think the code is perfectly valid and understandable. I have way more problems with the description in the title. I wouldn't understand the problem just by that and i doubt that the code does what the title asks for. Commented Oct 15, 2024 at 14:36
  • 1
    IMO, this is as simple as it can get. Its also clear to other developers as to what you are doing, Other fancy way might make other dev (or yourself even, in a few month) go ???, wondering what the code is actually doing (or why its more complicated). Commented Oct 15, 2024 at 14:44

6 Answers 6

Reset to default
5

I could recommend below alternative approach, with specifying helper method to easify syntax

public bool Update() {
    if (SpecialConditionChanged())
        UpdateSpecialCondition();
    
    bool[] results = [
        UpdateConditionIf(Condition1Changed, UpdateCondition1),
        UpdateConditionIf(Condition2Changed, UpdateCondition2),
        ...
    ];

    return results.Any();
}

private static bool UpdateConditionIf(
    Func<bool> predicate,
    Action updateAction
)
{
    var result = predicate();
    if(result) updateAction();
    return result;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

@ipodtouch0218 Woah, my miss, thanks for spotting!
@ipodtouch0218 Hm it will return true if there is at least one true, which is what we want here. Also, all items in array will be materialisd, it's not IEnumerable. It will evaluate all items
CheckConditionAndUpdate - How about UpdateConditionIf ? The update shall only be executed if the condition actually resolves to true, CheckCondition**And**Update - at least to me - implies both be executed, no matter what.
@Fildor Good idea, i like its semantic :)
But it would also be kind of cool to be actually able to adress dvers by @downvoter (at least once).
|
4

You could create a list with the pair of condition/update functions. And then iterate that.

public bool Update() 
{
    bool shouldNotify = false;

    var conditions = new List<(Func<bool> conditionChanged, Action updateAction)> 
    {
        (Condition1Changed, UpdateCondition1),
        (Condition2Changed, UpdateCondition2),
        ...
    };

    if (SpecialConditionChanged()) 
    {
        UpdateSpecialCondition();
    }

    foreach (var (conditionChanged, updateAction) in conditions) 
    {
        if (conditionChanged()) 
        {
            updateAction();
            shouldNotify = true;
        }
    }

    return shouldNotify;
}
Improve this answer

Comments

3

You can use a local function:

public bool Update()
{
    if (SpecialConditionChanged()) UpdateSpecialCondition();

    bool shouldNotify = false;

    void IfChangedUpdateAndNotify(Func<bool> changed, Action update)
    {
        if (changed()) { update(); shouldNotify = true; }
    }

    IfChangedUpdateAndNotify(Condition1Changed, UpdateCondition1);
    IfChangedUpdateAndNotify(Condition2Changed, UpdateCondition2);

    return shouldNotify;
}
Improve this answer

1 Comment

Upvoting this answer as being the clearest code, as well as most succinct.
1

Another way is using a variable number of arguments, which are tuples from a condition function and an updating action:

public bool Update()
{
    if (SpecialConditionChanged())
        UpdateSpecialCondition();

    return Update(
        (Condition1Changed, UpdateCondition1),
        (Condition2Changed, UpdateCondition2));
}

public bool Update(params (Func<bool> cond, Action update)[] condUpdPairs)
{
    var updates = from pair in condUpdPairs where pair.cond() select pair.update;
    foreach (var update in updates) update();
    return updates.Any();
}
Improve this answer

Comments

1

An another solution consists in using a record to define conditions, combined with LINQ for filtering and updating. This approach reduces repetitive code and makes it easier to add or modify conditions.

The Condition record encapsulates the logic for determining if a condition has changed (HasChanged), the update action (Update), and whether a notification is required (ShouldNotify). LINQ is then used to filter and update the changed conditions.

This way, 'special conditions' are handled just like any other.

public record Condition(Func<bool> HasChanged, Action Update, bool ShouldNotify);

public bool Update()
{
    var conditions = new List<Condition>
    {
        new Condition(SpecialConditionChanged, UpdateSpecialCondition, false),
        new Condition(Condition1Changed, UpdateCondition1, true),
        new Condition(Condition2Changed, UpdateCondition2, true),
    };

    return conditions
        // Select conditions that have changed
        .Where(c => c.HasChanged())  
        // Update each condition and return it
        .Select(c => { c.Update(); return c; })  
        // Return true if any condition requires notification
        .Any(c => c.ShouldNotify);  
}
Improve this answer

Comments

0

I ended up cherry picking bits from all of these solutions, so I wanted to share what I ended up with. This is a little closer to the actual code, but still slightly paraphrased.

public bool Update<T>(T target, T source)
{
    target.UpdateConditionIfDifferent(source, "SpecialCondition");

    var otherConditions = new string[] { "Condition1", "Condition2", ... }

    var wasEdited = otherConditions.Select(s => target.UpdateConditionIfDifferent(source, s));

    return wasEdited.Any();
}

public static bool UpdateConditionIfDifferent<T>(this T target, T source, string condition)
{
    var targetCondition = target.GetType().GetProperty(condition);
    var sourceCondition = source.GetType().GetProperty(condition);

    if (sourceCondition.GetValue(source) != targetCondition.GetValue(target)
        && targetCondition.CanWrite)
    {
        targetCondition.SetValue(target, sourceCondition.GetValue(source));
        return true;
    }
    
    return false;
}
Improve this answer

1 Comment

wasEdited.Any(p => p) ?

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.