Return to Answer

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

Some other ideas to consider:

1. 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.

1. 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).

2. 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;
 }

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

Some other ideas to consider:

1. 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.

1. 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).

2. 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;
 }

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

Some other ideas to consider:

1. 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.

1. 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).

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

Some other ideas to consider:

1. 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.

1. 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).

2. 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;
 }

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

I don't understand exactly the purpose of the code but having two similar object inheritance tree (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:

1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<Part> parts = new ArrayList<Part>();
   

2. Public fields usually leads to harder maintenance. See: What's the deal with Java's public fields?

Some other ideas to consider:

1. 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.

1. 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).