Refactoring multiple non-nested if statements [C#]

Question

I have a number of non-nested if statements that look like this:

if (!bytes[nameof(PropertyVersion.Price)].SequenceEqual(dbBytes[nameof(PropertyVersion.Price)])) changes += $"Price {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.Price, propertyVersion.Price)} from: £{dbPropertyVersion.Price:N0}. ";
if (!bytes[nameof(PropertyVersion.PriceMin)].SequenceEqual(dbBytes[nameof(PropertyVersion.PriceMin)])) changes += $"Min Price {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.PriceMin, propertyVersion.PriceMin)} from: £{dbPropertyVersion.PriceMin:N0}. ";
if (!bytes[nameof(PropertyVersion.PriceMax)].SequenceEqual(dbBytes[nameof(PropertyVersion.PriceMax)])) changes += $"Max Price {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.PriceMax, propertyVersion.PriceMax)} from: £{dbPropertyVersion.PriceMax:N0}. ";
if (!bytes[nameof(PropertyVersion.PriceQualifier)].SequenceEqual(dbBytes[nameof(PropertyVersion.PriceQualifier)])) changes += $"Price qualifier updated from: {dbPropertyVersion.PriceQualifier}. ";
if (!bytes[nameof(PropertyVersion.ChainFree)].SequenceEqual(dbBytes[nameof(PropertyVersion.ChainFree)])) changes += $"Chain updated from: {dbPropertyVersion.ChainFree}. ";
if (!bytes[nameof(PropertyVersion.Description)].SequenceEqual(dbBytes[nameof(PropertyVersion.Description)])) changes += "Description updated. ";
if (!bytes[nameof(PropertyVersion.Tenure)].SequenceEqual(dbBytes[nameof(PropertyVersion.Tenure)])) changes += $"Tenure updated from: {dbPropertyVersion.Tenure}. ";
if (!bytes[nameof(PropertyVersion.Auction)].SequenceEqual(dbBytes[nameof(PropertyVersion.Auction)])) changes += $"Auction updated from: {dbPropertyVersion.Auction}. ";
if (!bytes[nameof(PropertyVersion.ImageCount)].SequenceEqual(dbBytes[nameof(PropertyVersion.ImageCount)])) changes += $"Image count {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.ImageCount, propertyVersion.ImageCount)} from: {dbPropertyVersion.ImageCount}. ";
if (!bytes[nameof(PropertyVersion.ImageUrl)].SequenceEqual(dbBytes[nameof(PropertyVersion.ImageUrl)])) changes += "Image URL updated. ";
if (!bytes[nameof(PropertyVersion.VideoCount)].SequenceEqual(dbBytes[nameof(PropertyVersion.VideoCount)])) changes += $"Video count {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.VideoCount, propertyVersion.VideoCount)} from: {dbPropertyVersion.VideoCount}. ";
if (!bytes[nameof(PropertyVersion.VideoUrl)].SequenceEqual(dbBytes[nameof(PropertyVersion.VideoUrl)])) changes += "Video URL updated. ";
if (!bytes[nameof(PropertyVersion.FloorPlanCount)].SequenceEqual(dbBytes[nameof(PropertyVersion.FloorPlanCount)])) changes += $"Floorplan count {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.FloorPlanCount, propertyVersion.FloorPlanCount)} from: {dbPropertyVersion.FloorPlanCount}. ";
if (!bytes[nameof(PropertyVersion.FloorPlanUrl)].SequenceEqual(dbBytes[nameof(PropertyVersion.FloorPlanUrl)])) changes += "Floorplan URL updated. ";
if (!bytes[nameof(PropertyVersion.EpcRatingCount)].SequenceEqual(dbBytes[nameof(PropertyVersion.EpcRatingCount)])) changes += $"EPC certificate count {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.EpcRatingCount, propertyVersion.EpcRatingCount)} from: {dbPropertyVersion.EpcRatingCount}. ";
if (!bytes[nameof(PropertyVersion.EpcRatingUrl)].SequenceEqual(dbBytes[nameof(PropertyVersion.EpcRatingUrl)])) changes += "EPC certificate URL updated. ";
if (!bytes[nameof(PropertyVersion.BrochureCount)].SequenceEqual(dbBytes[nameof(PropertyVersion.BrochureCount)])) changes += $"Brochure count {TextHelper.AssessValueIncreasedOrDecreased(dbPropertyVersion.BrochureCount, propertyVersion.BrochureCount)} from: {dbPropertyVersion.BrochureCount}. ";
if (!bytes[nameof(PropertyVersion.BrochureUrl)].SequenceEqual(dbBytes[nameof(PropertyVersion.BrochureUrl)])) changes += "Brochure URL updated. ";
if (!bytes[nameof(PropertyVersion.ViewCount30Days)].SequenceEqual(dbBytes[nameof(PropertyVersion.ViewCount30Days)])) changes += "View count updated. ";

As you can see, each if statement is essentially checking for the same condition, and appends some text to a string variable if the condition is not met.

My question is, what design patterns can I utilise to not make the code look so messy?

(I've tried refactoring the if logic into a method and that doesn't really look much more appealing than how it is currently)

Does this answer your question? [Approaches for checking multiple conditions?](https://softwareengineering.stackexchange.com/questions/191208/approaches-for-checking-multiple-conditions) — gnat, Sep 29 '22 at 18:35
I would ask at codereview.SE, not here. If you do so, please delete your question here *before* you post it there, crossposts will not be accepted by our community. — Doc Brown, Sep 29 '22 at 18:54
While I think this could fit on Code Review, it is only a partial snippet of code. I attempted a conceptual answer anyhow. — Greg Burghardt, Sep 29 '22 at 20:58
It looks like PropertyVersion is an enum. You could iterate over the values of the enum and have a single if statement in a foreach instead. Yet I am wondering whether this change tracking can be easier and more effectively done using an event handler that is called at the time of a change. — Martin Maat, Sep 30 '22 at 05:31

score 3 · Answer 1 · answered Sep 29 '22 at 20:57

This might be solvable with polymorphism. It would still be crazy, but it could be an option.

Roughly, it looks like the logic is:

if (bytes equals dbBytes)
    changes += "some sort of text";

General repetition like this can largely be eliminated by encapsulating this logic in its own class or sub classes, which hopefully would result in code that looks like this.

First, you will need an abstract base class:

abstract class PropertyChecker
{
    public abstract bool SequenceEquals(byte[] bytes, byte[] dbBytes);
    public abstract string FormatChangesText(/* params here */);
}

Then a single sub class of PropertyChecker for each check that you need to do, implementing the two abstract methods. Lastly, you can eliminate the repeated if-statements in favor of a collection and a loop (or LINQ in this case):

var properties = new List<PropertyChecker>()
{
    new PricePropertyChecker(),
    new PriceMinPropertyChecker(),
    new PriceMaxPropertyChecker(),
    new PriceQualifierPropertyChecker(),
    // ...
};

var propertyChanges = from p in properties
                      where p.SequenceEquals(bytes, dbBytes)
                      select p.FormatChangesText(TextHelper, dbPropertyVersion, propertyVersion);
//                  (use consistent params here) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

changes += string.Join(" ", propertyChanges);

The important bits are:

Each class implements its own bool SequenceEquals(byte[], byte[]) method.
Each class implements its own string FormatChangesText(...)
- This method might take more parameters than each individual property checker needs, but it should pass everything they all need as a whole.

Many times a bunch of similar-looking if statements can be refactored into a collection of objects and a loop, as long as each if-statement is sufficiently similar.

`SequenceEquals` looks like it can be implemented in the base class with a `string PropertyName` member or somesuch — Caleth, Sep 30 '22 at 09:10

score 0 · Answer 2 · answered Oct 02 '22 at 16:43

Since all the statements are almost identical, you would use a macro in languages like C or C++, or an inline function otherwise. So your first two If statements could become

Update(Price, ”Price”)
Update(PriceMin, “Min Price”)

Inline functions are less flexible because they must be syntactically correct.

score 0 · Answer 3 · answered Oct 02 '22 at 18:52

Your code seems to require the items be checked in a particular order (e.g. Price has to come before PriceMin), so there needs to be some sort of data structure that declares that order.

Also, the text for each item is different, so there needs to be some sort of data structure to contain those.

Since the text sometimes contains fields that need to be filled, we can implement it as a function.

All this can be stored in a an ordered dictionary:

var dictionary = new OrderedDictionary<string,Func<object,object,string>>
{
    { nameof(PropertyVersion.Price), (old,new) => $"Price changed from {old} to {new}" },
    { nameof(PropertyVersion.PriceMin), (old,new) => $"Price minimum changed from {old} to {new}" },
    { nameof(PropertyVersion.Description), (old,new) => $"Description updated" },
    //Add additional rows per property
};

Once you have that, the code is easy:

foreach (var item in dictionary)
{
    var oldValue = bytes[item.Key];
    var newValue = dbBytes[item.Key];
    if (oldValue != newValue)
    {
        changes += item.Value(oldValue,newValue);
    }
}

Refactoring multiple non-nested if statements [C#]

3 Answers3