5
\$\begingroup\$

I have a class that creates an object, PlannedYear, and then passes that object into several other classes to manipulate it. The ultimate goal of the class is to manipulate and add items to this PlannedYear object.

I have, however, ended up going down a rabbit hole whereby I am updating the same PlannedYear object in memory in multiple classes, rather than having methods with return types and so on.

Can anyone look at my code and advise me if I can (or if I even need to) get out of the rabbit hole? I have simplified the code to demonstrate the issue and added comments.

EDIT: The overall purpose of the code is to read an existing production plan from a database, then take new items (stored in Estimate objects) and schedule them on top.

Planner.cs:

public class Planner
{
    // This object ends up getting modified all over the place
    // Ultimately it is this YearPlan that is the output of this class
    public PlannedYear YearPlan { get; private set; } 

    private PlanReader planReader = new PlanReader();

    public Planner()
    {
        YearPlan = planReader.GetExistingScheduleFromDatabase();
    }

    public void Plan(List<Estimate> estimates)
    {
        foreach (var estimate in estimates.OrderBy(x => x.Priority))
        {
            // Here's the start of the issue. YearPlan passed into a scheduler and gets modified
            var scheduler = new EstimateScheduler(estimate, YearPlan); 
            scheduler.Schedule();
        }
    }
}

EstimateScheduler.cs:

public class EstimateScheduler
{
    private PlanSearcher planSearcher;
    private Estimate estimateToSchedule;

    public EstimateScheduler(Estimate estimate, PlannedYear yearPlan)
    {
        estimateToSchedule = estimate;
        // yearPlan passed into planSearcher
        planSearcher = new PlanSearcher(yearPlan, 
                                        estimateToSchedule.StartDate, 
                                        estimateToSchedule.EndDate);
    }

    public void Schedule()
    {
        foreach (var itemHeader in estimateToSchedule.ItemHeaders)
        {
            ScheduleItemHeader(itemHeader);
        }
    }

    private void ScheduleItemHeader(EstimateItemHeader itemHeader)
    {
        foreach (var item in itemHeader.EstimateItems)
        {
            if (!planSearcher.DoesABedBigEnoughForThisItemExist(item.Length, item.Width))
            {
                HandleItemThatCannotBePlanned(item); // Not important
            }
            else
            {
                var canItemBeAddedToBed = true;
                while (item.HasItemsRemainingToBeScheduled && canItemBeAddedToBed)
                {
                    // get a bed from planSearcher (which searches yearPlan) and try to add an item to it
                    var bed = planSearcher.FindBedSpaceForItem(item);
                    if (bed != null)
                    {
                        bed.AddItem(item); // Updates the actual bed object which persists in planSeacher
                        item.ItemsScheduled++;
                    }
                    else
                    {
                        canItemBeAddedToBed = false;
                        HandleItemThatCannotBePlanned(item);
                    }
                }
            }
        }
    }
}

PlanSearcher.cs:

public class PlanSearcher
{
    private List<PlannedDay> plannedDays;

    public PlanSearcher(PlannedYear yearPlan, DateTime? start, DateTime? end)
    {
        // we take yearPlan and get only the range of days we need to modify
        plannedDays = yearPlan.PlannedDays.Where(x => x.Date >= start && x.Date <= end).ToList();
    }

    public PlannedBed FindBedSpaceForItem(IItem item)
    {
        // search this.plannedDays for a bed and return it
    }
}

PlannedDay.cs:

public class PlannedDay
{
    public DateTime Date { get; set; }
    public List<PlanendBed> PlannedBeds { get; set; }

    public PlannedDay(DateTime date)
    {
        this.Date = date;
        this.PlannedBeds = new List<PlannedBed>();
    }
}

PlannedYear.cs:

public class PlannedYear
{
   public List<PlannedDay> PlannedDays { get; set; }

   public PlannedYear()
   {
      PlannedDays = new List<PlannedDay>();
   }
}
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks! \$\endgroup\$ – Toby Speight Nov 27 '19 at 17:18
  • 1
    \$\begingroup\$ @TobySpeight Hi Toby. I have edited the question to describe the overall purpose of the code is. I am not sure if I can add this to the title in a way that is clear and not confusing. \$\endgroup\$ – MSOACC Nov 27 '19 at 17:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.