Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Redundant condition check before assignment suggestion for C# in Resharper 5

Is the condition check really redundant in the following sample?:

public class MyClass     {
    public bool MyProperty { get; set; }

    public void DoSomething(bool newValue) {
        // R# says: redundant condition check before assignment
        // on the following line:
        if (MyProperty != newValue) { // <======
            MyProperty = newValue;
        }
    }
}

I know that either way MyProperty will be set to newValue, but is the check redundant?

In Adobe Flex, the getter is called implicitly by the VM its running on whenever a setter is called even though no explicit check is being made. The end result is that checking before an assignment results in two checks, one explicit and one implicit, resulting in a redundant check. Does anything similar happen in C#?

like image 809
Kaleb Pederson Avatar asked Jan 20 '11 20:01

Kaleb Pederson


3 Answers

There are only two situations where I've seen this type of check.

The first is when there is an additional line of code which sets another property on the object to True to indicate that the object has been modified. This is typically used when trying to decide whether to persist the state of the object to something like a database.

The second situation is when the types in question are immutable. You might want to avoid setting the value and therefore creating a new string, for example, when the values are the same. Even then, I've only seen it in certain apps where memory usage is critical.

like image 139
NotMe Avatar answered Nov 20 '22 20:11

NotMe


In this specific case, it's logically redundant, since there is no code being executed in the getter - just a straight wrapper around a private field. If you're in the habit of putting stuff in your getter that would have side effects, I'd say to disable that R# warning.

Might be worth trying to put something in the getter of the property, and see if ReSharper still thinks it's redundant. If it does, then I'd call that a R# bug.

like image 4
Joe Enos Avatar answered Nov 20 '22 18:11

Joe Enos


I would say that the check is redundant. It would make more sense if you had an implementation of INotifyPropertyChanged, but then the check would be in the setter to avoid triggering the event if no actual change is done.

like image 1
sisve Avatar answered Nov 20 '22 18:11

sisve