CODEWITHSUNDEEP
c#
Mediator
Mediator

Reputation: 15378

How optimize by lambda expression

I have a very similar function is only one previous report and the other future, how can I optimize and write beautiful?

public bool AnyPreviousReportByGroup(int groupID)
        {
            if(this.GroupID == groupID)
            {
                return true;
            }
            else
            {
                return PreviousReport.AnyPreviousReportByGroup(groupID);
            }
        }

        public bool AnyNextReportByGroup(int groupID)
        {

            if (this.GroupID == groupID)
            {
                return true;
            }
            else
            {
                return NextReport.AnyNextReportByGroup(groupID);
            }
        }

Upvotes: 0

Views: 441

Answers (5)

Douglas
Douglas

Reputation: 54887

The following code is a conciser way of achieving the same thing:

public bool AnyPreviousReportByGroup(int groupID)
{
    return this.GroupID == groupID ||
           this.PreviousReport != null &&
           this.PreviousReport.AnyPreviousReportByGroup(groupID);
}

If you really want to use lambda expressions, here's a possible way:

public bool AnyReportByGroup(int groupID, Func<Report, Report> getOtherReport)
{
    if (this.GroupID == groupID)
        return true;

    Report other = getOtherReport(this);
    return other != null &&
           other.AnyReportByGroup(groupID, getOtherReport);
}

You could then call this helper method using lambda expressions:

bool anyPrevious = this.AnyReportByGroup(groupID, report => report.PreviousReport);
bool anyNext = this.AnyReportByGroup(groupID, report => report.NextReport);

Upvotes: 1

weston
weston

Reputation: 54791

Standard way to traverse linked lists is like so:

    public bool AnyPreviousReportByGroup(int groupID)
    {
        var item = this;
        do
        {
            if (item.GroupId == groupID)
            {
                return true;
            }
            item = item.PreviousReport;
        } while (item != null);
        return false;
    }

    public bool AnyNextReportByGroup(int groupID)
    {
        var item = this;
        do
        {
            if (item.GroupId == groupID)
            {
                return true;
            }
            item = item.NextReport;
        } while (item != null);
        return false;
    }

This has a benefit of not creating potentially massive call stacks the way a recusive approach would.

This also fixes your code, where it never returned false, it would just NPE.

Now we can refactor as you requested:

    private bool AnyReportByGroup(int groupID, bool searchForward)
    {
        var item = this;
        do
        {
            if (item.GroupId == groupID)
            {
                return true;
            }
            item = searchForward ? item.NextReport : item.PreviousReport;
        } while (item != null);
        return false;
    }

    public bool AnyPreviousReportByGroup(int groupID)
    {
        return AnyReportByGroup(groupID, false);
    }

    public bool AnyNextReportByGroup(int groupID)
    {
        return AnyReportByGroup(groupID, true);
    }

Upvotes: 0

Rawling
Rawling

Reputation: 50114

Here's a non-recursive solution, assuming that you want to return false when you run out of reports:

public bool AnyPreviousReportByGroup(int groupID)
{
    return GetEventualValue(groupID, r => r.PreviousReport);
}

public bool AnyNextReportByGroup(int groupID)
{
    return GetEventualValue(groupID, r => r.NextReport);
}

public bool GetEventualValue(int groupID, Func<Report, Report> nextReport)
{
    Report report = this;
    while (report != null && report.GroupID != groupID)
    {
        report = nextReport(report);
    }
    return report != null;
}

Upvotes: 0

Dennis
Dennis

Reputation: 37770

private bool AnyReportByGroup(int groupID, Func<int, bool> callback)
{

    if (this.GroupID == groupID)
    {
        return true;
    }
    else
    {
        return callback(groupID);
    }
}

public bool AnyPreviousReportByGroup(int groupID)
{
    return AnyReportByGroup(groupID, gid => PreviousReport.AnyPreviousReportByGroup(gid));
}

public bool AnyNextReportByGroup(int groupID)
{
    return AnyReportByGroup(groupID, gid => NextReport.AnyNextReportByGroup(gid));
}

But, I hope, that these methods are just a sample, and in your real code they're more complex.
Otherwise, I can't understand, what do you try to optimize.

Upvotes: 1

yogi
yogi

Reputation: 19601

May be like this

public enum ReportType{ Next, Previous }

public bool AnyReportByGroup(int groupID, ReportType type)
{
  if(this.GroupID == groupID)
      return true;
  else
  {
      switch(type)
      {
          case ReportType.Next:
            return NextReport.AnyNextReportByGroup(groupID);
          case ReportType.Previous:
            return NextReport.AnyPreviousReportByGroup(groupID);
      }
      return false;
  }
}

Upvotes: 0

Related Questions