1 2 Previous Next 17 Replies Latest reply: Aug 30, 2010 2:47 PM by 843853 RSS

    Need help with Factory Pattern.

    843853
      Hello Everyone!

      Guys, I have the following structure: parent class TransportType -> child class Car, child class Bike, child class Boat etc. Each of the child classes has its own constructor with a specific number of parameters. For example: Car(color=red, wheels = 3), Boat(Sail = true) etc

      The question is: what is the best way to create the class factory? I can see 3 ways:
      1) TransportFactory.createCar(String carColor, int carWheels);
      2) TransportFactory.createTransport(String transportClass, Object[] transportParams);
      3) TransportFactory.createTransport(Object[] allParams);

      In item 1 I'll have to create a number of functions, one for each child class. Moreover, I'll have to change my code each time a new child class is added.
      In item 2 and 3 I can use the Reflection API to create the object instead of multiple if-then switches. This will decrease the amount of code. But I can use TransportFactory.getDeclaredConstructors() only in case all the child classes are located inside TransportFactory class. This will make my code a bit complicated. Also I have heard from senior developers that I should avoid using Reflection if possible.

      So, what is the best way to implement this structure?
        • 1. Re: Need help with Factory Pattern.
          jschellSomeoneStoleMyAlias
          Dymytry wrote:
          Hello Everyone!

          Guys, I have the following structure: parent class TransportType -> child class Car, child class Bike, child class Boat etc. Each of the child classes has its own constructor with a specific number of parameters. For example: Car(color=red, wheels = 3), Boat(Sail = true) etc

          The question is: what is the best way to create the class factory? I can see 3 ways:
          1) TransportFactory.createCar(String carColor, int carWheels);
          2) TransportFactory.createTransport(String transportClass, Object[] transportParams);
          3) TransportFactory.createTransport(Object[] allParams);

          Moreover, I'll have to change my code each time a new child class is added.
          And when you add a new child class that is code change anyways.

          I would probably use 3. Assuming I used a factory at all.

          As a note your parameters suggest that you do not have a true class hierarchy.
          • 2. Re: Need help with Factory Pattern.
            800387
            Dymytry wrote:
            Hello Everyone!

            Guys, I have the following structure: parent class TransportType -> child class Car, child class Bike, child class Boat etc. Each of the child classes has its own constructor with a specific number of parameters. For example: Car(color=red, wheels = 3), Boat(Sail = true) etc
            My hope is you have TransportType as an interface and not an abstract class, especially since different constructors will apparently be used for the implementations.
            The question is: what is the best way to create the class factory? I can see 3 ways:
            1) TransportFactory.createCar(String carColor, int carWheels);
            2) TransportFactory.createTransport(String transportClass, Object[] transportParams);
            3) TransportFactory.createTransport(Object[] allParams);
            You lose type safety with options 2 and 3. If you insist on a factory, then create a static method for each instance to return.
            In item 1 I'll have to create a number of functions, one for each child class. Moreover, I'll have to change my code each time a new child class is added.
            Such is the nature of the beast. Unless you want to simply have Map<Object, Object> to hold the variables, I do not see a way around this.
            In item 2 and 3 I can use the Reflection API to create the object instead of multiple if-then switches. This will decrease the amount of code. But I can use TransportFactory.getDeclaredConstructors() only in case all the child classes are located inside TransportFactory class. This will make my code a bit complicated. Also I have heard from senior developers that I should avoid using Reflection if possible.
            Reflection is a bit slower. You again lose type safety and likely some refactoring assistance that might be available from your IDE.
            So, what is the best way to implement this structure?
            See above.

            - Saish
            • 3. Re: Need help with Factory Pattern.
              843853
              Guys, thank you for replies!

              Here are some comments:

              1) Well, actually my TransportType class is an abstract class because I have several functions that are common for all the children. Is it bad? Why?
              2) Yes, I need to store my Transports somehow. I have created a map <params[], TransportType> where params[] are: the name of class + input parameters for the corresponding constructor. This doesn't look neat, but at the moment I dont know how can I rearrange this.
              3) I dont insist on factory, but what is the work around? Factory or not, anyway I need a function such as "getTransport(transportType, transportParam1...transportParamN)" which checks my Map if such a object already exists, and creates it if doesn't.

              Any other suggestions?

              Edited by: Dymytry on Apr 20, 2010 6:17 AM
              • 4. Re: Need help with Factory Pattern.
                800387
                Dymytry wrote:
                Guys, thank you for replies!

                Here are some comments:

                1) Well, actually my TransportType class is an abstract class because I have several functions that are common for all the children. Is it bad? Why?
                In general, avoid inheritance where possible, particularly concrete inheritance. Abstract inheritance is fine if you are implementing an interface and there is common functionality (e.g., placing a concrete public, protected or package-private method in the superclass) or a given set of methods will be called in order for all subclasses (e.g., declare a protected or package-private abstract method in the super-class that sub-classes must implement).

                Why avoid inheritance? It is a scarce resource. You only get to inherit (from a class) once. So, all things being equal, if you can accomplish the same task with composition and delegation (e.g., moving the common functionality into a helper object), avoid inheritance.
                2) Yes, I need to store my Transports somehow. I have created a map <params[], TransportType> where params[] are: the name of class + input parameters for the corresponding constructor. This doesn't look neat, but at the moment I dont know how can I rearrange this.
                I'm not sure why you are not simply creating an interface TransportType. Place that in a collection or map. You really should only need a Map<String, Object> or whatever when receiving values from an external source (say, a user or another system) to create a TransportType. Ideally, this code is in a controller class (see model-view-controller pattern).
                3) I dont insist on factory, but what is the work around? Factory or not, anyway I need a function such as "getTransport(transportType, transportParam1...transportParamN)" which checks my Map if such a object already exists, and creates it if doesn't.
                You can always use the 'new' keyword.
                Any other suggestions?

                Edited by: Dymytry on Apr 20, 2010 6:17 AM
                - Saish
                • 5. Re: Need help with Factory Pattern.
                  843853
                  Saish, can you clarify your idea:
                  Saish wrote:
                  I'm not sure why you are not simply creating an interface TransportType. Place that in a collection or map. You really should only need a Map<String, Object> or whatever when receiving values from an external source (say, a user or another system) to create a TransportType.
                  Do you suggest to create a String key from all constructor parameters and thus form a map: HashMap<String, TransportType> ? This is the same thing I have now, except for the key which is a Object[] in my case.
                  • 6. Re: Need help with Factory Pattern.
                    800387
                    No, I am saying have your Factory (and the TransportType instances) have constructors that are strongly-typed. Meaning, as an example, CarTransportType(final String model, final String make, final Color color, final int year). You would have a controller (a Servlet or a web service or whatever) read from a Map<String, String>, since that is how HTTP requests come in. You would in that controller convert strings to integers, strings to colors, etc. Then call your factory or the constructor of a given TransportType itself.

                    - Saish
                    • 7. Re: Need help with Factory Pattern.
                      jschellSomeoneStoleMyAlias
                      Dymytry wrote:
                      1) Well, actually my TransportType class is an abstract class because I have several functions that are common for all the children. Is it bad? Why?
                      As expressed definitely so.

                      You do not use inheritence for shared behavior. You use it because a "is-a" relationship exists.
                      • 8. Re: Need help with Factory Pattern.
                        800387
                        You use it because a "is-a" relationship exists.
                        And for most things you model, there is more than one "is-a".

                        - Saish
                        • 9. Re: Need help with Factory Pattern.
                          843853
                          So, now I have:
                                Car getCar(int wheels, Color color, int doors) {
                                    
                                    Car car;
                          
                                    //form key
                                    Object[] key = new Object[] {"Car", wheels, color, doors};
                          
                                    if (tMap.containsKey(key)) {
                                         car = (Car)tMap.get(key); }
                                    else {
                                         car = new Car(wheels, color, doors);
                                         tMap.put(key, car);
                                    }
                                    
                                    return car;
                               }
                          And I'll have to repeat that code for 6 or 7 times with slight changes.

                          Guys, I still wonder is that style of coding is really better than one Reflection function for all? I thought OOP was invented to reduce the number of copy-pasting..

                          Edited by: Dymytry on Apr 21, 2010 1:50 AM
                          • 10. Re: Need help with Factory Pattern.
                            800387
                            There are dozens of ways you could implement this. Here is just one idea:
                            public final class TransportTypeFactoryTest {
                            
                                @Test
                            
                                public final void testFactoryLookupFromMap() {
                            
                                    final Map<String, String> test = new HashMap<String, String>();
                                    test.put("type", "Car");
                                    test.put("color", "black");
                            
                                    TransportType type = TransportTypeFactory.newInstance(test);
                                    assert (type instanceof Car);
                            
                                    test.clear();
                                    test.put("type", "Boat");
                                    test.put("sail", "true");
                                    type = TransportTypeFactory.newInstance(test);
                                    assert (type instanceof Boat);
                                }
                            }
                            
                            interface TransportType {
                            
                                public abstract void drive();
                            }
                            
                            final class Car implements TransportType {
                            
                                private static final String NAME = "Car";
                            
                                private final String color;
                            
                                Car(final String color) {
                            
                                    this.color = color;
                                }
                            
                                public final void drive() {
                            
                                    System.out.println("The " + color + " car is driving");
                                }
                            
                                public final String getColor() {
                            
                                    return color;
                                }
                            
                                static final boolean likes(final String name) {
                            
                                    return name.equals(NAME);
                                }
                            }
                            
                            final class Boat implements TransportType {
                            
                                private static final String NAME = "Boat";
                            
                                private final boolean sail;
                            
                                Boat(final boolean sail) {
                            
                                    this.sail = sail;
                                }
                            
                                public final void drive() {
                            
                                    System.out.println("Boat is " + (sail ? "sailing" : "motoring"));
                                }
                            
                                static final boolean likes(final String name) {
                            
                                    return name.equals(NAME);
                                }
                            }
                            
                            final class TransportTypeFactory {
                            
                                private TransportTypeFactory() {}
                            
                                public static final TransportType newInstance(final Map<String, String> params) {
                            
                                    final String type = params.get("type");
                                    assert (type != null) : "Missing required parameter 'type'";
                                    if (Car.likes(type)) {
                                        return newCar(params);
                                    }
                                    if (Boat.likes(type)) {
                                        return newBoat(params);
                                    }
                                    throw new IllegalArgumentException("Parameter 'type' has unknown value '" + type + "'");
                                }
                            
                                private static TransportType newCar(final Map<String, String> params) {
                            
                                    final String color = params.get("color");
                                    assert (color != null) : "Missing requried parameter 'color'";
                                    return new Car(color);
                                }
                            
                                private static TransportType newBoat(final Map<String, String> params) {
                            
                                    final String sail = params.get("sail");
                                    assert (sail != null) : "Missing required parameter 'sail'";
                                    return new Boat(Boolean.parseBoolean(sail));
                                }
                            }
                            - Saish
                            • 11. Re: Need help with Factory Pattern.
                              jschellSomeoneStoleMyAlias
                              Dymytry wrote:
                              I thought OOP was invented to reduce the number of copy-pasting..
                              That would be incorrect.

                              And using that as a sole or primary criteria for either procedural or OO programming (and probably others) would probably lead to maintenance problems.
                              • 12. Re: Need help with Factory Pattern.
                                843853
                                Saish, jshell thank you very much!
                                I'll study Saish's code and will proceed in this way.
                                • 13. Re: Need help with Factory Pattern.
                                  800387
                                  You are welcome. If you want to use the assertions in production (we do, but many companies leave them off), you need to start up your application or container with the -ea switch (enable assertions). If you still want the checks but do not want to leave assertions on, replace the assertions with an if statement that throws an exception on a validation error. Best of luck.

                                  - Saish
                                  • 14. Re: Need help with Factory Pattern.
                                    843853
                                    This doesn't look neat, but at the moment I dont know how can I rearrange this.

                                    [Wedding Alterations|http://www.MinaDesignAndTailoring.com]
                                    1 2 Previous Next