CODEWITHSUNDEEP
c#optimization
dstewart101
dstewart101

Reputation: 1112

how can i reduce the duplication in these methods

Consider the following method:

    private bool DescriptionValid(Membership membership, string identifier)
    {
    // search for identifier in 4 lists
    // only need to find it failing in one

    if (membership.premium.Where(ev => ev.Id == identifier).Any())
    {
        var ev = membership.premium.Where(x => x.Id == identifier).Select(m => m).SingleOrDefault();
        if (String.IsNullOrEmpty(ev.Remarks))
        {
            return false;
        }
    }

    if (membership.club.Where(ev => ev.Id == identifier).Any())
    {
        var ev = membership.club.Where(x => x.Id == identifier).Select(m => m).SingleOrDefault();
        if (String.IsNullOrEmpty(ev.Remarks))
        {
            return false;
        }
    }

    if (membership.basic.Where(ev => ev.Id == identifier).Any())
    {
        var ev = membership.basic.Where(x => x.Id == identifier).Select(m => m).SingleOrDefault();
        if (String.IsNullOrEmpty(ev.Remarks))
        {
            return false;
        }
    }

    if (membership.junior.Where(ev => ev.Id == identifier).Any())
    {
        var ev = membership.junior.Where(x => x.Id == identifier).Select(m => m).SingleOrDefault();
        if (String.IsNullOrEmpty(ev.Remarks))
        {
            return false;
        }
    }

    // no fails
    return true;
}

Membership is a property which contains four lists:

public IList<PremiumMemberShip> premium { get; set; } = new List<PremiumMemberShip>();

public IList<ClubMemberShip> club { get; set; } = new List<ClubMemberShip>();

public IList<BasicMemberShip> basic { get; set; } = new List<BasicMemberShip>();

public IList<JuniorMemberShip> junior { get; set; } = new List<JuniorMemberShip>();

Each membership is different, but they share similar traits. Is there a way I can condense the code below? Essentially the same property is being tested each time, it's just that the type is changing for the different ifs.

It feels like there should be a better way to do this.

Upvotes: 1

Views: 75

Answers (2)

Enigmativity
Enigmativity

Reputation: 117009

Without changing any of the structure of you code you could always just do this:

private bool DescriptionValid(Membership membership, string identifier)
{
    var query =
        from ms in new []
        {
            membership.premium.Select(m => new { m.Id, m.Remarks }),
            membership.club.Select(m => new { m.Id, m.Remarks }),
            membership.basic.Select(m => new { m.Id, m.Remarks }),
            membership.junior.Select(m => new { m.Id, m.Remarks }),
        }
        let ev = ms.Where(x => x.Id == identifier).SingleOrDefault()
        where ev != null && String.IsNullOrEmpty(ev.Remarks)
        select ev;

    return !query.Any();
}

Upvotes: 2

usr
usr

Reputation: 171178

In general you can create common code by keeping the commonalities and abstracting from the differences. Create a helper method:

  1. The source collection (membership.premium) becomes a method argument.
  2. The return false; becomes a return type of bool.
  3. The classes (PremiumMemberShip, ...) must expose a common API surface to the helper. This can be done in various ways:
    1. An interface with Id and Remarks members.
    2. A common base class.
    3. dynamic.
    4. The helper method could be generic and take functions Func<T, int> getID, Func<T, string> getRemarks.

Otherwise, the helper method contains a copy of the existing code with fairly mechanical changes.

Upvotes: 1

Related Questions