Is below implementation of Abstract Factory Design Pattern is correct?

Question

    #include<iostream>

    using namespace std;

    //Abstract Product A

    class Document
    {

        public:

            virtual void createDocument() = 0;

    };


    class Word: public Document

    {

        public :


            virtual void createDocument()
            {

                cout<<"Document Word is created"<<endl;         

            }
    };


    class Excel : public Document
    {
        public :

            virtual void createDocument()
            {
                cout<<"Document Excel is created"<<endl;            
            }
    };

    //Abstract Product B


    class VideoFile
    {

        public:

            virtual void createVideoFile() = 0;

    };


    class VLC : public VideoFile
    {

        public :

            virtual void createVideoFile()
            {
                cout<<"VideoFile VLC is created"<<endl;         

            }
    };


    class AVI : public VideoFile
    {

        public :

            virtual void createVideoFile()
            {

                cout<<"VideoFile AVI is created"<<endl;         

            }
    };



    //////////////////////////////////////////////////////////////////////


    class AbstractFactory

    {

        public:

            virtual Document* createDocFiles(string fileType) = 0;

            virtual VideoFile* createVideoFiles(string fileType) = 0;       

    };


    class ConcreteFactoryA : public AbstractFactory
    {

        public:

            virtual Document* createDocFiles(string fileType)
            {

                if(fileType == "Word")
                    return new Word();
                if(fileType == "Excel")
                    return new Excel();
            }

            virtual VideoFile* createVideoFiles(string fileType)
            {

                if(fileType == "VLC")
                    return new VLC();
                if(fileType == "AVI")
                    return new AVI();
            }

    };


    class ConcreteFactoryB : public AbstractFactory
    {

        public:

            virtual Document* createDocFiles(string fileType)
            {

                if(fileType == "Word")
                    return new Word();
                if(fileType == "Excel")
                    return new Excel();
            }

            virtual VideoFile* createVideoFiles(string fileType)
            {

                if(fileType == "VLC")
                    return new VLC();
                if(fileType == "AVI")
                    return new AVI();
            }

    };



    int main()
    {

        AbstractFactory *ptrAbFac = new ConcreteFactoryB();

        Document *ptrDoc =  ptrAbFac->createDocFiles("Excel");

        ptrDoc->createDocument();

        VideoFile  *ptrVideoFile = ptrAbFac->createVideoFiles("VLC");

        ptrVideoFile->createVideoFile();

    }

user4581301 · Answer 1 · 2018-07-31T19:29:38.867

You are close, but the Abstract Product classes (Document and derived classes and VideoFile and derived classes) should not have create methods. The factory does the creating and the object returned should be ready to use.

This may just be poor selection of method name, but should be corrected to prevent confusion. For the sake of your example, you can simply rename the function to DoSomething so that you have a method you can call to demonstrate success. Example:

class Document
{
    public:
        virtual ~Document() = default; // must have virtual destructor
        virtual void doSomething() = 0;
};

class Word: public Document
{
    public :
        void doSomething() override // no  need for virtual. 
                                    // override helps prevent mistakes
        {
            cout<<"Document Word"<<endl;
        }
};

Side notes

The Abstract Products and AbstractFactory need virtual destructors. Discussion of why here: When to use virtual destructors?.

In the derived classes you don't need to redeclare that the methods are virtual. Once a method is declared virtual in a base class, all overrides are automatically virtual. Also, get used to using the override keyword for methods that should override. Very useful in preventing errors should the interface change and leave you with methods that no longer override.

Don't forget to delete all those objects the factories provided. Prefer std::unique_ptr if it is available to you.

createDocFiles and createVideoFiles (both implementations) do not return in all possible paths. Eg: Nothing is returned from createDocFiles if the provided fileType is "Word Perfect". Compilers will usually flag this, but allow the program to compile (unless you've set warnings as errors). If a program compiles with warnings, odds are good that the program doesn't do what you want it to do. Fix:

        virtual Document* createDocFiles(string fileType)
        {
            if(fileType == "Word")
                return new Word();
            if(fileType == "Excel")
                return new Excel();
            throw std::runtime_error("Unknown file type");
        }

Thanks for your suggestions . I have a query that which approach is better To implement Abstract Factory, Condition based or Template based — Monty Singh, Aug 01 '18 at 02:53
Not sure what you mean there, @MontySingh . I recommend asking a new question so you can fully flesh out what you want to know. — user4581301, Aug 01 '18 at 04:04
template(class T) class ConcreteFactoryA : public AbstractFactory { public: virtual Document* createDocFiles() { return new T(); } }; As in factory class instead of pass argument to function createDocFile. We can use above template approach. Above question is regarding the same. — Monty Singh, Aug 01 '18 at 08:35

Is below implementation of Abstract Factory Design Pattern is correct?

1 Answers1

Side notes