patternjavaMinor
Factory for classes unknown at compile time
Viewed 0 times
compilefactoryunknowntimeforclasses
Problem
I have a class called
An action will perform the same manipulation for each
Neither
```
import java.util.ArrayList;
public class GenericsTest
{
public static void main(String[] args) throws InstantiationException,
IllegalAccessException
{
ArrayList parts = new ArrayList();
Rotate rotate = ActionFactory.getAction(Rotate.class);
Move move = ActionFactory.getAction(Move.class);
int i;
for (i = 0; i parts = new ArrayList();
}
abstract class Part
{
// Raw type warning
public ArrayList actions = new ArrayList();
}
abstract class Action
{
abstract public void execute(T part);
}
class ActionFactory
{
public static T getAction(Class c) throws InstantiationException,
IllegalAccessException
{
T returnAction = null;
for (Action action : actions)
{
if (action.getClass() == c)
Machine. It contains a list of parts and a list of actions. The list of parts will contain instances of the Part class subclasses and the list of actions will contain instances of my Action class subclasses. Each Part subclass will have its corresponding Action subclass. None of the subclasses are available at compile time. When a Machine instance starts all its parts should perform their action.An action will perform the same manipulation for each
Part subclass so I am trying to create a factory class that will create a single instance for each Action subclass. My first thought was to use an interface for Action, but then I would have to create an Action instance for each Part instance I create, or make each class that implemented Action responsible for making itself a singleton. I am not sure if a factory is the best solution and if I am implementing the pattern correctly.Neither
Action nor Part subclasses are available to me so I am using generics. I am new to Java so I am not sure if I am using them correctly - Eclipse gives me several Raw type warnings (I indicated where). Here is my code:```
import java.util.ArrayList;
public class GenericsTest
{
public static void main(String[] args) throws InstantiationException,
IllegalAccessException
{
ArrayList parts = new ArrayList();
Rotate rotate = ActionFactory.getAction(Rotate.class);
Move move = ActionFactory.getAction(Move.class);
int i;
for (i = 0; i parts = new ArrayList();
}
abstract class Part
{
// Raw type warning
public ArrayList actions = new ArrayList();
}
abstract class Action
{
abstract public void execute(T part);
}
class ActionFactory
{
public static T getAction(Class c) throws InstantiationException,
IllegalAccessException
{
T returnAction = null;
for (Action action : actions)
{
if (action.getClass() == c)
Solution
I don't understand exactly the purpose of the code but having two similar object inheritance tree (
Other notes:
-
-
Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?
Some other ideas to consider:
-
Create a separate
Store the actions in a list and then you can call
It's type-safe, clients can't create an
-
I'd move the
-
If the
I guess you still need a class which stores the actions which a machine should do:
Actions and Parts) smells a little bit. It also seems a sample code and I guess it violates the single responsibility principle (the behaviour of a part is separated to two classes) and it might also violates the Liskov substitution principle too (if you have an Action you can't use it with any kind of Part). I'd consider moving the action code to the Part class or creating generic Action classes which ignore unhandled Parts (by class type). Anyway, it's hard to say something more useful without the purpose of the code. See also: SOLID, Parallel Inheritance Hierarchies.Other notes:
-
ArrayList reference types should be simply List. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfacesList parts = new ArrayList();-
Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?
Some other ideas to consider:
-
Create a separate
Action instance for every part and action. Pass the Part instance to the constructor of your Action. For example:Action rotate = new Rotate(gear);Store the actions in a list and then you can call
execute on the rotate instance:for (final Action action: actions) {
action.execute();
}It's type-safe, clients can't create an
Action with a wrong Part type.-
I'd move the
actions list from the Part object to somewhere else. Why should a part know what actions does a machine do with it? It seems to me that this responsibility should be stored somewhere else. The Macine is the closest object now (but it might deserve separate class).-
If the
Action classes do not have any state consider moving their code inside the Part object. (The basic idea of OOP is that the data and the logic which operates with the data should be together in a class.) You can mark the action method with an @Action annotation and parse it runtime. public class Gear {
...
@Action
public void rotate() {
...
}
}I guess you still need a class which stores the actions which a machine should do:
public class Action {
private Part part;
private String actionMethodName;
}Code Snippets
List<Part> parts = new ArrayList<Part>();Action rotate = new Rotate(gear);for (final Action action: actions) {
action.execute();
}public class Gear {
...
@Action
public void rotate() {
...
}
}public class Action {
private Part part;
private String actionMethodName;
}Context
StackExchange Code Review Q#41725, answer score: 4
Revisions (0)
No revisions yet.