4
\$\begingroup\$

I need to know if my code is a valid implementation of the factory method pattern. Also, if the Configuration and Editor were interfaces would this still be a valid implementation of the Factory Pattern?

public abstract class Configuration {

    protected List<String> keyWords;

    abstract void getTheme();
    abstract String getKeyWords();
}
public abstract class Editor {

    public final void useEditor() {
        Configuration configuration = createConfiguration();
        configuration.getTheme();
        parse(configuration.getKeyWords());
        save();
    }

    //The Factory Method
    public abstract Configuration createConfiguration();

    public abstract void parse(String sourceCode);

    public void save() {
        System.out.println("The file was saved..");
    }
}
public class JavaConfiguration extends Configuration {
    public JavaConfiguration() {
        keyWords = new ArrayList<>();
        keyWords.add("class");
        keyWords.add("protected");
    }

    @Override
    void getTheme() {
        System.out.println("Applied Java Theme...");
    }

    @Override
    String getKeyWords() {
        return keyWords.toString();
    }
}
public class PythonConfiguration extends Configuration {

    public PythonConfiguration() {
        keyWords = new ArrayList<>();
        keyWords.add("def");
        keyWords.add("print");
    }

    @Override
    void getTheme() {
        System.out.println("Applied Python Theme...");
    }

    @Override
    String getKeyWords() {
        return keyWords.toString();
    }
}
public class JavaEditor extends Editor {
    @Override
    public Configuration createConfiguration() {
        return new JavaConfiguration();
    }

    @Override
    public void parse(String sourceCode) {
        System.out.println("Parsing Java code...");
    }
}
public class PythonEditor extends Editor {

    @Override
    public Configuration createConfiguration() {
        return new PythonConfiguration();
    }

    @Override
    public void parse(String sourceCode) {
        System.out.println("Parsing python code...");
    }
}
public class App {

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        String language = sc.next();
        Editor editor = createEditor(language);
        editor.useEditor();
    }

    private static Editor createEditor(String language) {
        switch(language) {
            case "Java":
                return new JavaEditor();
            case "Python":
                return new PythonEditor();
            default:
                return null;
        }
    }
}

Edit: It seems that the Factory Method Pattern is not the appropriate design pattern to use in this case, as suggested by @200_success and @Timothy Truckle. A simple Editor class which receives a Configuration as a parameter should be enough.

\$\endgroup\$
3
\$\begingroup\$

I need to know if my code is a valid implementation of the factory method pattern.

No.

The reason is that the factory cannot be part of the objects to be created. I think you would be surprised to find a factory in the the hedge of your car, wouldn't you?

Generally spoken your approach fails the Separation of Concerns principle.

You have two responsibilities in your code that belong to a factory:

  1. identifying the requested type.
  2. creating the actual object.

Both should be in a separate factory class.


Also: your "factory" is producing the wrong type of objects because of your misconception of inheritance.

We employ inheritance when the child classes differ in behavior. But your Editor extensions only differ in configuration (as far as your code example is showing...).

So what your (yet to build) factory should do is creating the "language depended" key word list and pass that into a new instance of the one and only Editor class.

\$\endgroup\$
2
\$\begingroup\$

The getKeyWords() method is not particularly useful. It takes a List<String> and summarizes its contents in a degraded form. It's particularly inappropriate for Editor.useEditor() to attempt to parse this undocumented string representation. I would prefer to have it return an unmodifiable list. The JavaConfiguration class would be better written as

import java.util.Arrays;
import java.util.Collections;

public class JavaConfiguration extends Configuration {
    private static List<> KEYWORDS = Collections.unmodifiableList(Arrays.asList(
        "class",
        "protected"
    ));

    @Override
    List<String> getKeywords() {
        return KEYWORDS;
    }

    …
}

Does the order of the keywords matter? Perhaps a Set would be more appropriate than a List.


Typically, you wouldn't want to have the App be responsible for performing the translation from language name to the associated Configuration object.

public abstract class Configuration {
    abstract void getTheme();
    abstract List<String> getKeywords();

    public static Configuration forLanguage(String language) {
        switch (language) {
            case "Java":
                …
        }
    }
}

A factory might not be the most appropriate pattern, though, since there isn't any point in instantiating many copies of each Configuration subclass. Reusing singletons might be better.


One would hope that the whole point of having various Configurations is to contain all the language-specific behaviours of the Editor. Therefore, if the system is designed properly, then there should only be one Editor class, whose constructor accepts a Configuration. (I'm assuming that you don't need the Editor to be able to switch languages on the fly.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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