CODEWITHSUNDEEP
asp.net
John C Scott
John C Scott

Reputation: 146

nice overload structure

I have created a library function and want to add an overload that does a very similar thing with an additional parameter. The existing code looks like this:

public class MealsAllocation
{
    public int mealId;
    public List<CrewSummary> crew;

    private MealsAllocation() { }
    public MealsAllocation(int MealId) {
        mealId = MealId;
        string connStr = ConfigurationManager.ConnectionStrings["LocalSqlServer"].ConnectionString;
        SqlConnection conn = new SqlConnection(connStr);

        //first fill an ienumerable redemption object for the meal
        List<MealRedemption> mealRedemptions = new List<MealRedemption>();
        SqlCommand cmdRed = new SqlCommand("tegsGetMealsRedemption", conn);
        cmdRed.CommandType = CommandType.StoredProcedure;
        cmdRed.Parameters.Add(new SqlParameter("@mealId", MealId));
        conn.Open();
        SqlDataReader drRed = cmdRed.ExecuteReader();
        while (drRed.Read())
        {
            MealRedemption mr = new MealRedemption(Convert.ToInt32(drRed["crewId"]), Convert.ToDateTime(drRed["creation"]), Convert.ToInt32(drRed["redeemed"]));
            mealRedemptions.Add(mr);
        }
        conn.Close();

        //then fill the crew list
        crew = new List<CrewSummary>();
        SqlCommand cmdCrew = new SqlCommand("tegsGetMealsAllocation", conn);
        cmdCrew.CommandType = CommandType.StoredProcedure;
        cmdCrew.Parameters.Add(new SqlParameter("@mealId", MealId));
        conn.Open();
        SqlDataReader drCrew = cmdCrew.ExecuteReader();
        while (drCrew.Read())
        {
            int drCid = Convert.ToInt32(drCrew["id"]);
            List<MealRedemption> drMr = mealRedemptions.FindAll(red => red.crewId == drCid) ;
            CrewSummary cs = new CrewSummary(drCid, Convert.ToInt32(drCrew["allocation"]), drMr );
            crew.Add(cs);
        }
        conn.Close();

    }

So then now I wish to add a new overload that will look a bit like this:

    public MealsAllocation(int MealId, int crewId)
    {
    }

and essentially this will do much the same but slightly different from the above.

What would be a good strategy to avoid "copy and paste inheritance" ? ie a nice way to refactor the above so that it lends itself more easily to the overload?

Upvotes: 0

Views: 61

Answers (5)

Steve
Steve

Reputation: 216313

The first thing that comes to mind is to split that big chunk of code in two different methods giving at each method a specialized functionality

public MealsAllocation(int MealId) 
{
    List<MealRedemption> mealRedemptions = LoadMealRedemptions(MealID);
    LoadCrewSummaryByMeal(mealRedemptions, MealID);
}

while the other constructor could be

public MealsAllocation(int MealId, int crewId) 
{ 
    List<MealRedemption> mealRedemptions = LoadMealRedemptions(MealID);
    LoadCrewSummaryByCrew(mealRedemptions, MealID, crewID);
}

In the first constructor you call the private method where you load the MealRedemptions list, get its output and pass to a specialized method that load the CrewSummary list using only the MealID and the list obtained from the first method.

In the second constructor you could use the same method used in the first one and then use a different one for the loading of the CrewSummary. The requirements of your second constructor are not clear and could change the design of this second method (I mean, how do you use the crewID parameter to change the inner workings to build the CrewSummary list?)

Upvotes: 1

Chris Gessler
Chris Gessler

Reputation: 23123

Although I don't recommend doing all that inside the constructor, you can simply add an optional param to the end:

public class MealsAllocation  
{  
    public int MealId { get; set; }
    public int CrewId { get; set; }

    public List<CrewSummary> Crew { get; set; };

    public MealsAllocation(int mealId, int crewId = 0)  
    {  
        this.MealId = mealId;
        this.CrewId = crewId;

        if(this.CrewId = 0) // etc...
}

Side note: You need to add the using statement around your SqlConnection, SqlCommand and SqlDataReader object or you could run into connection and/or memory leaks. Personally, I'd create a Data Access layer and put all of the data related methods there to make them reusable across your entire business layer.

Also, I think this might be a good candidate for the Lazy<T> object: http://msdn.microsoft.com/en-us/library/dd642331.aspx

Upvotes: 1

Stephan Bauer
Stephan Bauer

Reputation: 9249

Since you want to overload your constructor, you could also try such an approach:

public MealsAllocation(int MealId) : this (MealId, null)
{
}
public MealsAllocation(int MealId, int? crewId) 
{
  // Initialize your instance as needed
  if (crewId.HasValue)
  {
    // Do some more stuff
  }
}

Upvotes: 1

Aghilas Yakoub
Aghilas Yakoub

Reputation: 29000

You can user object initializer

var mealRedemption = new
{ 
  MealId = yourvlue,
  Crew = crew
};

Link : http://msdn.microsoft.com/en-us/library/bb384062.aspx

Upvotes: 1

Paul Aldred-Bann
Paul Aldred-Bann

Reputation: 6020

How about moving your logic to an internal function so it's only accessible in this assembly, and to this class and use optional parameters... something like this:

public class MealsAllocation
{
    public int mealId;
    public List<CrewSummary> crew;

    private MealsAllocation() 
    {
    }

    public MealsAllocation(int MealId) 
    {
        DoWork(MealId);
    }

    public MealsAllocation(int MealId, int crewId)
    {
        DoWork(MealId, crewId);
    }

    internal void DoWork(int MealId, int crewId = -1)
    {
        // have your logic here based on your parameter list

        // valid crewId passed, then add new param for DB proc
        if (crewId > -1)
        {
            cmdCrew.Parameters.Add(new SqlParameter("@crewId", crewId));
        }
    }
}

Upvotes: 1

Related Questions