0
\$\begingroup\$

I am implementing the factory method approach. My intention is to know if my approach is correct for factory method. Any modifications to be done to align with the approach?

First I am creating an interface IAuto.

 //IAuto.cs
    namespace TestDesignPatterns.Auto
    {
        public interface IAuto
        {

            void start();
            void stop();
        }
    }

Create a class that implements the IAuto.

    //clsBMW.cs
    namespace TestDesignPatterns.Auto
    {
        public class clsBMW : IAuto
        {
            public void start()
            {
                Console.WriteLine("BMW started");
            }

            public void stop()
            {
                Console.WriteLine("BMW stopped");
            }
        }
    }

Create another concrete class for Audi implementing IAuto.

namespace TestDesignPatterns.Auto
{
    public class clsAudi : IAuto
    {
        public void start()
        {
            Console.WriteLine("Audi started");
        }

        public void stop()
        {
            Console.WriteLine("Audi stopped");
        }
    }
}

Create an Interface of factory which creates the model.

//IautoFactory.cs
public interface IAutoFactory
    {
       IAuto createModel(ModelType m);
    }

concrete factory class.

//clsAutofactory.cs
 public class clsAutoFactory : IAutoFactory
    {
        public IAuto createModel(ModelType m)
        {
            IAuto instance = null;
            switch (Convert.ToInt32(m))
            {
                case 0:
                    instance = new clsBMW();
                    break;

                case 1:
                   instance =  new clsAudi();
                    break;
                default:
                    break;


            }

            return instance;
        }
    }

Main Program

 public enum ModelType
    {
        BMW=0,
        Audi=1
    }
//program.cs

            IAutoFactory factory = new clsAutoFactory();
            IAuto model = factory.createModel(ModelType.Audi);
            model.start();
            model.stop();
            Console.ReadKey();
\$\endgroup\$
4

2 Answers 2

4
\$\begingroup\$

Do not name things with data type prefixes

clsAutoFactory. No. Instead: AutoFactory

Names should be what they are in the problem domain - autos in this case. How a thing is implemented - is it an array? a class? an integer? etc. is not what's important, we can read code and the IDE is real smart. Writing code that is descriptive is the thing.


Unnecessary interfaces

There is no reason for any interfaces at this point. Just inherit from Auto class. Half the code goes away! Perhaps make it an abstract class and override methods as needed. I bet there is common code and/or default behavior.

In general interfaces are for giving unrelated classes similar behavior. A Parrot might implement the Idogable so it can bark like a dog even though a Parrot is not a Dog. But a BMW is an Auto.


I am implementing the factory method

You wrote a factory class not a factory method.

A factory method would be a method in some other class that does the factory thing. The code simple enough at this point that you could create a factory method - say a method in the Auto class; but I am not saying you should.

There are 3 "levels" of factory design which facilitates increasing construction complexity. i.e. factory method, factory (a simple class), and abstract factory for very complex things. The abstract factory may be appropriate if you were creating multiple auto makes with multiple models with multiple individual options; and perhaps different kinds like SUV, sports car, truck.


Ditto to what @JAD said


switch default

this is a perfect place for catching errors. If the default case is hit then obviously that model has not been implemented in your code. So capture that fact and let the user know.


Be more specific with auto instance names. model is too generic.

Not this:

IAuto model = factory.createModel(ModelType.Audi);
model.start();
model.stop();

but this:

IAuto myAudi = factory.createModel(ModelType.Audi);
myAudi.start();
myAudi.stop();
\$\endgroup\$
4
  • \$\begingroup\$ you mentioned "A factory method would be a method in some other class that does the factory thing". I am not clear on this. My understanding was the method createModel is a method that creates the object. \$\endgroup\$
    – krishna_v
    Commented Oct 9, 2018 at 4:42
  • \$\begingroup\$ made the code modifications as commented. Could you confirm if the approach is factory method and not factory class. I have added a method which returns the instance. Kindly correct me if i am wrong \$\endgroup\$
    – krishna_v
    Commented Oct 9, 2018 at 5:39
  • \$\begingroup\$ You should not alter original question code. At worst it could make all the comments and answers moot. At best it gets confusing. Stack Exchange cannot be helpful to others if answers look wrong or nonsensical because code was altered later. \$\endgroup\$
    – radarbob
    Commented Oct 9, 2018 at 6:00
  • \$\begingroup\$ public class clsAutoFactory. You said you wrote a "factory method" but the factory is written as an independent class. Either way, it's a "factory" - i.e. it builds objects. There is a technical distinction between a factory method (not written as it's own class) and a factory (as its own class). \$\endgroup\$
    – radarbob
    Commented Oct 9, 2018 at 6:14
2
\$\begingroup\$

switch (Convert.ToInt32(m)) this is counterproductive and hurts readability. You can use enums in switch statements, so instead do this:

switch (m)
{
    case ModelType.BMW:
        bla
    case ModelType.Audi:
        bla
}
\$\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.