3
\$\begingroup\$

I need a code review on my Abstract Factory pattern written in Java.

Find the two enums AnimalType.java and DietType.java below.

package com.patterns.abstractfactory.three;

public enum AnimalType {

TERRESTRIAL("\nLeave on land and breath with lungs.\n"), AQUATIC("\nLeave in water and breath with gills.\n");

private String demo;

AnimalType(String demo) {
    this.demo = demo;
}

@Override
public String toString(){
    return this.demo;
}
}




package com.patterns.abstractfactory.three;


public enum DietType {
CARNIVORE("\nFeed on flesh\n"), HERBIVORE("\nFeed on plants\n");

private String demo;

DietType(String demo){
    this.demo = demo;
}

@Override
public String toString(){
    return this.demo;
}
}

Animal.java Interface:

package com.patterns.abstractfactory.three;


public interface Animal {
void info();
}

Deer.java:

package com.patterns.abstractfactory.three;


public class Deer implements Animal{

private AnimalType at;

public Deer(AnimalType at){
    this.at = at;
}

@Override
public void info() {
    System.out.println(" This is Deer ");
    System.out.println(at);

}

}

Lion.java:

package com.patterns.abstractfactory.three;


public class Lion implements Animal{

private AnimalType at;

public Lion(AnimalType at){
    this.at = at;
}

@Override
public void info() {
    System.out.println(" This is beasty lion ");
    System.out.println(at);
}

}

Shark.java:

package com.patterns.abstractfactory.three;


public class Shark implements Animal{

private AnimalType at;

public Shark(AnimalType at){
    this.at = at;
}

@Override
public void info() {
    System.out.println(" This is Shark fish ");
    System.out.println(at);
}

}

Turtle.java:

package com.patterns.abstractfactory.three;

public class Turtle implements Animal{

private AnimalType at;

public Turtle(AnimalType at){
    this.at = at;
}

@Override
public void info() {
    System.out.println("This is sea turtle");
    System.out.println(at);
}

}

AnimalFactory.java:

package com.patterns.abstractfactory.three;

public interface AnimalFactory {
Animal createAnimal(DietType dietType) throws Exception;
}

LandAnimalFactory.java:

package com.patterns.abstractfactory.three;

public class LandAnimalFactory implements AnimalFactory{

private AnimalType at;

public LandAnimalFactory(AnimalType at){
    this.at = at;
}

@Override
public Animal createAnimal(DietType dietType) throws Exception {
    Animal animal = null;

    if(dietType == DietType.CARNIVORE){
        animal = new Lion(at);
    }else if(dietType == DietType.HERBIVORE){
        animal = new Deer(at);
    }else{
        throw new Exception(" Not able to create Land Animal  ");
    }

    return animal;
}


}

OceanAnimalFactory.java:

package com.patterns.abstractfactory.three;

public class OceanAnimalFactory implements AnimalFactory{

private AnimalType at;

public OceanAnimalFactory(AnimalType at){
    this.at = at;
}

@Override
public Animal createAnimal(DietType dt) throws Exception{
    Animal animal = null;

    if(dt == DietType.CARNIVORE){
        animal = new Shark(at);
    }else if(dt == DietType.HERBIVORE){
        animal = new Turtle(at);
    }else{
        throw new Exception(" Not able to create Ocean Animal ");
    }

    return animal;
}

}

AnimalTypeFactory.java:

package com.patterns.abstractfactory.three;

public abstract class AnimalTypeFactory {
public abstract AnimalFactory createFactory();
}

TerrestrialAnimalFactory.java:

package com.patterns.abstractfactory.three;

public class TerrestrialAnimalFactory extends AnimalTypeFactory{

@Override
public AnimalFactory createFactory() {
    return new LandAnimalFactory(AnimalType.TERRESTRIAL);
}

}

AquaticAnimalFactory.java:

package com.patterns.abstractfactory.three;

public class AquaticAnimalFactory extends AnimalTypeFactory {

@Override
public AnimalFactory createFactory() {
    return new OceanAnimalFactory(AnimalType.AQUATIC);
}
}

MainTestClass.java:

package com.patterns.abstractfactory.three;

public class MainTestClass {
public static void main(String [] args){
    Animal animal = null;
    AnimalTypeFactory animalTypeFactory = null;
    AnimalFactory animalFactory = null;        
    try{
        animalTypeFactory = new TerrestrialAnimalFactory();
        animalFactory =  animalTypeFactory.createFactory();
        animal = animalFactory.createAnimal(DietType.CARNIVORE);
        animal.info();
    }catch(Exception e){
        e.printStackTrace();
    }
}
}

From the above implementation of the Abstract Factory pattern, can you please review and tell me whether I have achieved it? If not, can you please tell me how I can improve my pattern in the code?

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

I didn't try to compile or run your code. These are my thoughts:

Important:

  1. When you do the Abstract Factory pattern, there are two pieces: the factory and the thing that creates the factory. You need to have a separate class that has a static method getFactory(). Here's a link that explains it: Abstract Factory Pattern instructions. If the diagram at the beginning is confusing then ignore it. Just follow the steps. You're definitely missing #7 but you should check the other ones as well.
  2. (Going along with my previous comment) The AnimalType should be passed as a parameter to a static method. Depending on the value of AnimalType a different factory is created.

Not important:

(This is my advice but others may disagree)

  1. It's better to use a switch statement than a series of if-then-else statements. (For example, LandAnimalFactory and OceanAnimalFactory. The default in the switch statement should be to throw an IllegalArgumentException. This will save you a lot of time when you are debugging. If your factory only knows how to create certain types of things, and the caller asks for something that you can't create, fail immediately. Otherwise it can be quite difficult to track down errors.

  2. Don't include \n in your strings, for example \nFeed on flesh\n. If the caller wants to print out a line break, let the caller print out a line break. There can be cases where including \n in a string is correct but in this case I think it makes the code harder to change when new requests come in. (I know this is probably a school assignment but it's still good to develop good habits.)

  3. It's better to throw IllegalArgumentException than Exception. (IllegalArgumentException is a subclass of RuntimeException so it doesn't need to be declared in the method signature.) The best time to throw checked exceptions (like Exception) is when the caller did what they were supposed to do but the method still didn't work.

  4. printStackTrace is fine for school but in the real world you'll probably want to do something else, like exiting the program or printing out a meaningful error message.

In summary: in MainTestClass.java, the part where you're creating the factory looks incorrect. Everything else looks good.

\$\endgroup\$
0
\$\begingroup\$

I originally wanted to write this as a comment, but I'll leave it as a code review, although I am not reviewing your whole code, but only the overall design of your example. Design is not a minor point.

I think your example is not appropriate for your purpose. When you create a carnivorous animal, it is a lion. You can't expand your example to include more animals which are carnivorous. Your design is upside down.

\$\endgroup\$

Your Answer

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

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