patternjavaModerate
Vegetable factory
Viewed 0 times
factoryvegetablestackoverflow
Problem
I'm new to Java. I'm trying to build a simple hierarchy of classes.
I have some abstract methods in class
I want to implement logic of buying vegetables. I've created the following class for it:
Is it a good way of doing that? And is it ideologically correct to name it factory? Any suggestions of improvement are highly appreciated.
abstract class Vegetable
{
public Vegetable(double weight)
{
this.weight = weight;
...
}
...
}
class Tomato extends Vegetable
{
public Tomato(double weight)
{
super(weight);
...
}
...
}
class Cucumber extends Vegetable
{
public Cucumber(double weight)
{
super(weight);
...
}
...
}I have some abstract methods in class
Vegetable, which are overridden in Tomato and Cucumber.I want to implement logic of buying vegetables. I've created the following class for it:
public class VegetableFactory
{
public static ArrayList buy(int num, Class vegetableClass)
{
ArrayList res = new ArrayList(num);
for (int i = 0; i < num; i++)
{
double weight = (Math.random() * 200) + 100;
Vegetable v = null;
try
{
v = (Vegetable) vegetableClass.getDeclaredConstructor(Double.TYPE).newInstance(weight);
} catch (InstantiationException e)
{
e.printStackTrace();
} catch (IllegalAccessException e)
{
e.printStackTrace();
} catch (InvocationTargetException e)
{
e.printStackTrace();
} catch (NoSuchMethodException e)
{
e.printStackTrace();
}
res.add(v);
}
return res;
}
}Is it a good way of doing that? And is it ideologically correct to name it factory? Any suggestions of improvement are highly appreciated.
Solution
I would use an enum type instead of having to pass a class in. This way, it's impossible to pass something bad in, and you can catch errors at compile time instead of runtime.
Then your buy method would be simply
Now of course the problem is that you end up having a big
The other thing that you could do is have a factory method on the
Then your list becomes
Finally, if you want to get fancy,
http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/reflect/Constructor.html
You can then use this to get the constructor to the
enum VegetableType { CARROT, TOMATO, CUCUMBER };Then your buy method would be simply
static ArrayList buy(int num, VegetableType vegType) {
ArrayList veggies = new ArrayList(n); //n is the initial capacity - size is still zero.
for(int i=0; i < num; ++i) {
double weight = (Math.random() * 200) + 100;
switch(vegType) {
case VegetableType.CARROT:
veggies.add(new Carrot(weight));
break;
case VegetableType.TOMATO:
veggies.add(new Tomato(weight));
break;
case VegetableType.CUCMBER:
veggies.add(new Cucumber(weight));
break;
}
}
return veggies;
}Now of course the problem is that you end up having a big
switch statement in the middle in your loop. Generally, while ugly I'd say that this is sufficient - you get some more control over each of the individual constructors, so you're not constrained to the same set of parameters.The other thing that you could do is have a factory method on the
VegetableType to create an instance:enum VegetableType {
CARROT { Vegetable createInstance(int weight) { return new Carrot(weight); }},
TOMATO { Vegetable createInstance(int weight) { return new Tomato(weight); } },
CUCUMBER { Vegetable createInstance(int weight) { return new Cucumber(weight); } };
abstract Vegetable createInstance(int weight);
}Then your list becomes
static ArrayList buy(int num, VegetableType vegType) {
ArrayList veggies = new ArrayList();
for(int i=0; i < num; ++i) {
double weight = (Math.random() * 200) + 100;
veggies.add(vegType.createInstance(weight));
}
return veggies;
}Finally, if you want to get fancy,
java.lang.reflect has the Constructor class:http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/reflect/Constructor.html
You can then use this to get the constructor to the
Vegetable type of your choice, and then call construct repeatedly in the loop.Code Snippets
enum VegetableType { CARROT, TOMATO, CUCUMBER };static ArrayList<Vegetable> buy(int num, VegetableType vegType) {
ArrayList<Vegetable> veggies = new ArrayList<Vegetable>(n); //n is the initial capacity - size is still zero.
for(int i=0; i < num; ++i) {
double weight = (Math.random() * 200) + 100;
switch(vegType) {
case VegetableType.CARROT:
veggies.add(new Carrot(weight));
break;
case VegetableType.TOMATO:
veggies.add(new Tomato(weight));
break;
case VegetableType.CUCMBER:
veggies.add(new Cucumber(weight));
break;
}
}
return veggies;
}enum VegetableType {
CARROT { Vegetable createInstance(int weight) { return new Carrot(weight); }},
TOMATO { Vegetable createInstance(int weight) { return new Tomato(weight); } },
CUCUMBER { Vegetable createInstance(int weight) { return new Cucumber(weight); } };
abstract Vegetable createInstance(int weight);
}static ArrayList<Vegetable> buy(int num, VegetableType vegType) {
ArrayList<Vegetable> veggies = new ArrayList<Vegetable>();
for(int i=0; i < num; ++i) {
double weight = (Math.random() * 200) + 100;
veggies.add(vegType.createInstance(weight));
}
return veggies;
}Context
StackExchange Code Review Q#8467, answer score: 14
Revisions (0)
No revisions yet.