开发者

Can I remove the property setter (without ANY problem) in the example provided in the question?

开发者 https://www.devze.com 2023-02-28 01:09 出处:网络
In existing code of my project, at number of places the property is declared like this: public long ResourceID

In existing code of my project, at number of places the property is declared like this:

public long ResourceID
{
    get
    {
        return this.resourceID;
    }
    set
    {
        if (this.resourceID != value)
        {
            this.resourceID = value;
        }
    }
}

Note: private long resourceID is already declared.

Properties not only of value types but also of reference types (including string) too are declared like this.

Another example:

public Collection<Ability> Abilities
{
    get
    {
        return this.abilities;
    }
    set
    {
        if (value == null)
        {
            throw new ArgumentNullException("Abilities");
        }
        this.abilities = value;
    }
}

As per my knowledge, the setter in the first example does not make any sense and the if condition is meaningless there. So i decided to change the code (as part of refactoring) to make开发者_运维问答 them Auto-Properties. (In second example I need setter since exception is handled there.)

I want to know from experts here, will whether making existing properties auto properties (or at least removing if condition from setter) cause any harm? Sometimes there are subtle things which a developer may not be aware of and some changes can have side effects too. That's why I am asking this question. (My libraries are used at many places.)

Note: Let me know if this is purely a homework question.


Converting:

private long resourceID;
public long ResourceID
{
    get
    {
        return this.resourceID;
    }
    set
    {
        this.resourceID = value;
    }
}

into:

public long ResourceID { get; set; }

won't cause any harm, guaranteed.

Removing the if statement might cause harm. For example in WPF when working with the MVVM pattern and implementing the INotifyPropertyChanged interface it is often good practice to check whether the value has changed before actually setting it. Removing this check will provoke notifications to be sent to the UI no matter whether the value changed or not. So it would be a breaking change.


I can only think of one kind of problem you could run into (which is fixable):

If you are using ORM or other external tool, they might rely on a naming convention for finding properties/fields. So, the 3rd party dll might be looking for a field resourceId that no longer exists.

So, code using reflection to access fields might break, but if you have control over the codebase, that is unlikely to be an issue.


There are some edge-cases where this might cause harm:

Changing to an automatically implemented property {get;set;}

if you are using field-based serialization at any point (for example, BinaryFormatter), then this will break when changing to an automatically implemented property, as the field-name will change. This will also impact any other scenario that uses reflection to access the (hopefully private) fields, but BinaryFormatter is the most common cause of confusion here.

Removing the if test

will be fine for most data-types such as long etc, however, if you use it with a type that implements a custom equality operation, you might find you are suddenly swapping a reference when previously (with the if) the objects reported equal for different references


The first is a more likely problem. If you are using BinaryFormatter, then keep the private field (byt maybe remove the if test). And then start refactoring your code away from BinaryFormatter ;p


What you have done is correct. The if statement is meaningless. I always think that less code is better, because the lines of code is directly proportional to the number of faults.

public long ResourceID { get; set; } 


Your first example only sets the resourceID field if its value has changed.

The only difference you would see by removing the "if" test is a possible impact if multiple threads are reading the value. In which case they probably should be using a lock, so it's almost certainly safe to remove the test.

Your second example prevents a caller from setting the property value to null. Presumably the field is initialized to a non-null value, and this has value as it means that callers can read the property without needing to check for null.


Usually in such scenarios and how you've explained, it shouldn't be a concern. You could just go ahead and change the code of all properties;

public long ResourceID { get; set; }

Or

public long ResourceID
{
    get { return this.resourceID; }

    set { this.resourceID = value; }
}
  • But it might cause an issue if upon changing the value of the property, it cascades to some other custom function-call which is only executed if the new value is different from old ones. Usually when when you've implemented custom events or even maybe in case of property-changed events
  • Also might affect, when using Data-Context classes

Both scenarios are totally application specific.

I'd suggest you reactor with caution. Or as you've written yourself, HOMEWORK.

0

精彩评论

暂无评论...
验证码 换一张
取 消