1
\$\begingroup\$

My abstract class. Product will have multiple child classes those will be different types of Product

 public abstract class Product
            {
                private int productId;
                public int ProductId
                {
                    get
                    {
                        return productId;
                    }
                    set
                    {
                        productId = value;
                    }
                }
        
                private double cost;
                public double Cost
                {
                    get
                    {
                        return cost;
                    }
                    set
                    {
                        cost = value;
                    }
                }
                protected IPayment payment;
                protected IEmail email;
                public string DoPayment()
                {
                    return payment.MakePayment();
                }
        
                public void setDifferentPaymentMethod(IPayment payment)
                {
                    this.payment = payment;
                }
            }
    

Concrete implementation of Product Class . How can I avoid instantiating PackingSlip here. Shall I create a factory?

 public class PhysicalProduct:Product
    {
        public PhysicalProduct()
        {
            payment = new PackingSlip();
        }
    }

Concrete implementation of Product Class

public class BookProduct:PhysicalProduct
    {
        public BookProduct():base()
        {
            payment = new DuplicatePackingSlip();           
        }
    }

Strategy class

 internal class DuplicatePackingSlip : IPayment
    {
        public string MakePayment()
        {
            return "Generating Duplicate Packing SLip for Royalty Department";
        }
    }

Strategy.

 internal class PackingSlip : IPayment
        {
            public string MakePayment()
            {
                return "Generating Packing Slip";
            }
        }

Is this the right way to implement if we want to have open/closed principle?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Indentation

Yours is highly inconsistent. It's best to use an autoformatter to ensure that this remains sane and consistent.

Property boilerplate

This:

            private int productId;
            public int ProductId
            {
                get
                {
                    return productId;
                }
                set
                {
                    productId = value;
                }
            }
    

is both very Java and unnecessary given the syntactic sugar that C# has supported for several years:

public int ProductId { get; set; }

Factory?

How can I avoid instantiating PackingSlip here

Why do you want to avoid instantiating it? Is it because PhysicalProduct is supposed to be an abstract class as well? If so, you are correct to want to avoid assigning an instance in the abstract class, since your assignment in the BookProduct constructor will overwrite this value.

There are several approaches you can follow. One is to make the top-level constructor accept an instance of IPayment, and to pass a new instance via base().

Another approach is to make the abstract base class a generic, where the generic type is forced to implement IPayment. The abstract class would not need to accept a new instance, and would be able to construct its own instance so long as the new constraint is imposed.

Redundant base call

This:

    public BookProduct():base()

should not need to explicitly call base.

MakePayment

One of two things has happened here. Either you've omitted a bunch of code, which would technically make this section of the question off-topic; or MakePayment is both misnamed and mischaracterized as a function. If the latter, make it a get-only property and rename it to something like Description.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.