How to clean up repetetive elements in a C# method?

Bender. 0
Viewed 79

I'm currently working on a .NET Core 3.1 application.

I need to refactor a code smell to reduce method complexity - in my method there are lots of repetitive elements.

All properties in my class are of type string. I need to check if the passed reference holds the same value as the property. If a referenced value differes, I need to set the flag to true and correct the Property value with the referenced one.

public string Name { get; set; }
// ... all Properties are of type String!

public bool MyMethod(Apple myApple)
{
    var flag = false;
    myApple.Id = Id;

    if (Name != myApple.Name)
    {
        Name = myApple.Name;
        flag = true;
    }
    if (Description != myApple.Description)
    {
        Description = myApple.Description;
        flag = true;
    }
    if (ShortDescription != myApple.ShortDescription)
    {
        ShortDescription = myApple.ShortDescription;
        flag = true;
    }
    if (Location != myApple.Location)
    {
        Location = myApple.Location;
        flag = true;
    }
    if (KindId != myApple.KindId)
    {
        KindId = myApple.KindId;
        flag = true;
    }
    if (Expiration != myApple.Expiration)
    {
        Expiration = myApple.Expiration;
        flag = true;
    }
    if (PurchaseDate != myApple.PurchaseDate)
    {
        PurchaseDate = myApple.PurchaseDate;
        flag = true;
    }

    return flag;
}

Do you know how to reduce complexity in my method and perhaps get rid of the repetetive elements?

class-library refactoring asp.net-core .net c#
4 Answers

You can shorten it a bit by introducing a local function and using one delegate to select the property to compare and another delegate to assign the property.

It would look something like this:

public bool MyMethod(Apple myApple)
{
    bool flag = false;

    void assignIfDifferent<T>(Func<Apple, T> selector, Action assign)
    {
        if (EqualityComparer<T>.Default.Equals(selector(myApple), selector(this)))
            return;

        assign();
        flag = true;
    }

    assignIfDifferent(apple => apple.Name,             () => { this.Name             = myApple.Name;             });
    assignIfDifferent(apple => apple.Description,      () => { this.Description      = myApple.Description;      });
    assignIfDifferent(apple => apple.ShortDescription, () => { this.ShortDescription = myApple.ShortDescription; });
    assignIfDifferent(apple => apple.Location,         () => { this.Location         = myApple.Location;         });

    // Etc etc.

    return flag;
}

I'm not really convinced that's an improvement, but that's for you to decide. It does at least reduce the number of lines of code.

Matthew Watson0

If you used fields instead of auto properties you could use the following pattern

private string name;
public string Name => name;
public bool MyMethod(Apple myApple)
{
    var flag = false;
    myApple.Id = Id;
    flag |= Set(ref name, myApple.Name);
    flag |= Set(ref description , myApple.Description);
    ...
}

private bool Set<T>(ref T field, T value) where T : IEquatable<T>{
    if(!field.Equals(value)){
        field = value;
        return true;
    }
    return false;
}

This method can also be combined with CallerMemberName to help raise events for wpf-code.

Doing the same pattern with properties is less neat since you need to either use delegates for setting the value, or reflection.

JonasH0

You can do it with using Reflection's. You can iterate all properties of class and control it.

var test = (new Apple()).MyMethod(new Apple() { Name = "Stack", Desc = "Test" });   


public class Apple
{
    public string Name { get; set; } = "Onur";
    public string Desc { get; set; } = "Test";
    // ... all Properties are of type String
    public bool MyMethod(Apple apple)
    {
        bool flag = false;
        IList<PropertyInfo> props = new List<PropertyInfo>(this.GetType().GetProperties());
        foreach (PropertyInfo prop in props)
        {
            try
            {
                //get value in method param
                object propValue = prop.GetValue(apple, null);
                //get current prop in current class
                PropertyInfo propResult = this.GetType().GetProperty(prop.Name, BindingFlags.Public | BindingFlags.Instance);
                var val = propResult.GetValue(this, null);
                if (val != propValue)
                {
                    propResult.SetValue(this, propValue, null);
                    flag = true;
                }
            }
            catch (Exception ex)
            {
            }
        }
        return flag;
    }
}
Onur Gelmez0

Another option could be to use something like a record for all the properties, i.e. just create a new record with all the new properties you want, then compare it to the old one to see if they are equal, using the auto generated equality method. i.e.

public record MyProperties(string Name, string Description);
...
private MyProperties props;
public string Name => props.Name;
...
public bool MyMethod(Apple myApple)
{
    var newProps = new MyProperties(
         myApple.Name, 
         myApple.Descrpition);
    var flag = newProps != props;
    props = newProps;
    return flag;
}
JonasH0
Related