Konstantin
Konstantin

Reputation: 35

Choose the right constructor

I have two classes: Purchase (the parent) and DiscountPurchase (child, with one additional field "discount"). Each one with 2 constructors (with and without parameters). I need parse string and create one of instances. I know, i can do it this way:

string[] parameters = csvString.Split(';');

string productName = parameters[0];
decimal cost = decimal.Parse(parameters[1]);
int productCount = int.Parse(parameters[2]);

if (parameters.Length < 4)
{                   
    newPurchase = new Purchase(productName, cost, productCount);
}
else
{
    decimal discount = decimal.Parse(parameters[3]);
    newPurchase = new FixedDiscountPurchase(productName, cost, productCount, discount);
}

But maybe there is some more elegant way: reflection or something else?

Upvotes: 2

Views: 238

Answers (4)

Tim S.
Tim S.

Reputation: 56536

Here's how you might do this using reflection. It works by trying to use the first constructor it sees with the right number of arguments. The list of types it searches is a type you give it plus any types that inherit directly from that type in the same assembly as that type.

It's pretty fragile, hard to read, and inflexible (e.g. if you add another type of purchase with three parameters on its constructor, it won't work right), which gives you an idea of what it's like to work with reflection. I do not recommend you do it this way.

static object CreateInstance(Type rootType, object[] args)
{
    var types = rootType.Assembly.GetTypes().Where(t =>
        t == rootType || t.BaseType == rootType).ToArray();
    return CreateInstance(types, args);
}
static object CreateInstance(Type[] types, object[] args)
{
    foreach (var type in types)
    {
        foreach (var ctor in type.GetConstructors())
        {
            var parameters = ctor.GetParameters();
            if (args.Length == parameters.Length)
            {
                var newArgs = args.Select((x, i) =>
                   Convert.ChangeType(x, parameters[i].ParameterType)).ToArray();
                return ctor.Invoke(newArgs);
            }
        }
    }
    return null;
}

// use like
var newPurchase = CreateInstance(typeof(Purchase), parameters); // or
var newPurchase = CreateInstance(
         new[] { typeof(Purchase), typeof(FixedDiscountPurchase) }, parameters);

Upvotes: 0

Icemanind
Icemanind

Reputation: 48686

I think everyone is over thinking this. If this is a brand new project that you are creating and down the road, there will be other types other than Purchase and DiscountPurchase, then dognose's answer about creating a factory might be the way to go.

Without having to tear up the code and do a lot of work though, an extension method might be the way to go:

public static Purchase GetPurchaseObject(this string csvString)
{
    string[] parameters = csvString.Split(';');

    string productName = parameters[0];
    decimal cost = decimal.Parse(parameters[1]);
    int productCount = int.Parse(parameters[2]);

    if (parameters.Length < 4)
    {
        return new Purchase(productName, cost, productCount);
    }
    else
    {
        decimal discount = decimal.Parse(parameters[3]);
        return new DiscountPurchase(productName, cost, productCount, discount);
    }
}

Then to use it, all you need to do is call the extension method from you CSV string:

string csvString1 = "TestProduct;15.50;5";
string csvString2 = "TestProduct;15.50;5;0.25";

Purchase p1 = csvString1.GetPurchaseObject();
Purchase p2 = csvString2.GetPurchaseObject();

if (p1 is DiscountPurchase)
{
    Console.WriteLine("p1 is a DiscountPurchase item");
}
else
{
    Console.WriteLine("p1 a Purchase item");
}

if (p2 is DiscountPurchase)
{
    Console.WriteLine("p2 is a DiscountPurchase item");
}
else
{
    Console.WriteLine("p2 a Purchase item");
}

As you can see from the output, p1 and p2 are different objects! Depending on the contents of the CSV string, it will know which kind of object you need. Also, if you do not like extension methods, you can move the logic into a static method and pass in a csv string as a parameter to the method.

Upvotes: 0

dognose
dognose

Reputation: 20899

This is a typical usecase for the factory-pattern. You know, that you need an Instance of Purchase - but you do not know the exact subtype. (See http://www.dotnetperls.com/factory and here for a slightly more complex and usefull example: http://msdn.microsoft.com/en-us/library/orm-9780596527730-01-05.aspx)

Ofc. this does not avoid the logic you already implemented - it just helps you that you don't have to repeat yourself over and over again, and have the logic encapsuled in one Factory you could use anywhere at any time.

Taken from your example, a simple factory might look like this:

static class PurchaseFactory
{
    public Static Purchase BuildPurchase(String[] parameters){
       string productName = parameters[0];
       decimal cost = decimal.Parse(parameters[1]);
       int productCount = int.Parse(parameters[2]);

       if (parameters.Length < 4)
       {                   
           return new Purchase(productName, cost, productCount);
       }
       else
       {
           decimal discount = decimal.Parse(parameters[3]);
           return new FixedDiscountPurchase(productName, cost, productCount, discount);
       }
    }
}

So, from anywhere within you code, you simply need to to:

string[] parameters = csvString.Split(';');
Purchase p = PurchaseFactory.BuildPurchase(parameters);

//p is now  either "Purchase" or "FixedDiscountPurchase"

Well, if it's just about these two classes and the wish to know if a price was discounted, or not (and caluclate the final price) - you could get away with one Purchase class that contains a discounted flag and a method to get the FinalPrice in any case:

public class Purchase
    {
        public Decimal Discount { get; set; }
        public Boolean Discounted { get; set; }
        public String Name { get; set; }
        public Decimal Price { get; set; }
        public Int32 Count { get; set; }

        public Decimal FinalPrice
        {
            get
            {
                if (!Discounted)
                    return Price;
                else
                    return Price - Discount;
            }
        }

        public Purchase (String csvString){
            string[] parameters = csvString.Split(';');

            Name = parameters[0];
            Price = decimal.Parse(parameters[1]);
            Count = int.Parse(parameters[2]);

            if (parameters.Length == 4)
            {
                Discount = decimal.Parse(parameters[3]);
                Discounted = true;
            }
        }
    }

usage:

Purchase p = new Purchase(stringInput);
MessageBox.Show(p.FinalPrice.ToString());

Just make sure to refer to purchase.FinalPrice, then you don't have to take care whether the price is actually discounted or not.

Upvotes: 1

Tim S.
Tim S.

Reputation: 56536

It might be better to redesign your Purchase class to have a discount field and just do something like this:

string[] parameters = csvString.Split(';');

string productName = parameters[0];
decimal cost = decimal.Parse(parameters[1]);
int productCount = int.Parse(parameters[2]);
decimal discount = parameters.Length < 4 ? 0 : decimal.Parse(parameters[3]);

newPurchase = new Purchase(productName, cost, productCount, discount);

Or keep the separate classes and change it so that the if works on whether there's a discount, instead of whether there's a fourth parameter. This adds a bit of decoupling between the logic of "where does this data come from" and "what do I do with it once I have it", which is a good thing.

string[] parameters = csvString.Split(';');

string productName = parameters[0];
decimal cost = decimal.Parse(parameters[1]);
int productCount = int.Parse(parameters[2]);
decimal discount = parameters.Length < 4 ? 0 : decimal.Parse(parameters[3]);

if (discount > 0)
{
    newPurchase = new FixedDiscountPurchase(productName, cost,
                                             productCount, discount);       
}
else
{
    newPurchase = new Purchase(productName, cost, productCount);
}

You could use decimal? and null (instead of 0) for the discount if you need to logically separate the difference between no discount being specified and a discount of 0 being specified.

Upvotes: 1

Related Questions