skiwi
skiwi

Reputation: 69249

I am needing super.super.method() -> Possible design flaw?

I have found myself to need an invocation of super.super.method() in java, which is not possible.

I am just wondering if I am having a design flaw here in my design, or not?

The classes:

package solvers.command;

/**
 *
 * @author student
 */
public abstract class Command {
    private boolean executed;   //executed state

    /**
     * Constructs a new Command object.
     * 
     * @modifies    this.executed = false
     */
    public Command() {
        this.executed = false;
    }

    /**
     * Executes this command.
     * 
     * @modifies executed = true
     * @pre {@code !executed}
     * @throws IllegalStateException if {@code executed}
     */
    public void execute() {
        if (executed) {
            throw new IllegalStateException("solvers.command.Command.execute: already executed");
        }
        executed = true;
    }

    /**
     * Undoes this command.
     * 
     * @modifies executed = false
     * @pre {@code executed}
     * @throws IllegalStateException if {@code !executed}
     */
    public void undo() {
        if (!executed) {
            throw new IllegalStateException("solvers.command.Command.undo: not executed yet");
        }
        executed = false;
    }

    /**
     * Returns the executed state
     * 
     * @return executed state
     */
    public boolean getExecuted() {
        return executed;
    }
}

package solvers.command;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
 *
 * @author student
 */
public class CompoundCommand extends Command {
    List<Command> commands; //list of commands

    /**
     * Creates a new CompoundCommand.
     * 
     * @modifies this.commands is initialised
     */
    public CompoundCommand() {
        super();
        this.commands = new ArrayList<>();
    }

    /**
     * Adds a command to the list of commands.
     * 
     * @param command   The new command
     * @pre {@code command != null}
     * @throws IllegalArgumentException if {@code command == null}
     */
    public void add(final Command command) {
        if (command == null) {
            throw new IllegalArgumentException("solvers.command.CompoundCommand.add: "
                    + "command == null");
        }
        commands.add(command);
    }

    /**
     * Removes a command from the list of commands.
     * 
     * @param command   The command to be removed
     * @pre {@code command != null && commands.contains(command}
     * @throws IllegalArgumentException if {@code command == null || !commands.contains(command)}
     */
    public void remove(final Command command) {
        if (command == null) {
            throw new IllegalArgumentException("solvers.command.CompoundCommand.remove: "
                    + "command == null");
        }
        if (!commands.contains(command)) {
            throw new IllegalArgumentException("solvers.command.CompoundCommand.remove:"
                    + "command is not found in commands");
        }
        commands.remove(command);
    }

    /**
     * Returns if the list of commands is empty.
     * 
     * @return {@code commands.isEmpty()}
     */
    public boolean isEmpty() {
        return commands.isEmpty();
    }

    @Override
    public void execute() {
        super.execute();
        for (Command c : commands) {
            c.execute();
        }
    }

    @Override
    public void undo() {
        super.undo();
        Collections.reverse(commands);
        for (Command c : commands) {
            c.undo();
        }
        Collections.reverse(commands);
    }
}

package solvers.command;

/**
 *
 * @author student
 */
public class ExecutedCompoundCommand extends CompoundCommand {

    /**
     * Creates a new ExecutedCompoundCommand.
     */
    public ExecutedCompoundCommand() {
        super();
    }

    @Override
    public void add(final Command command) {
        if (!command.getExecuted()) {
            throw new IllegalStateException("solvers.command.ExecutedCompoundCommand.add: "
                    + "command has not been executed yet.");
        }
        super.add(command);
    }

    @Override
    public void execute() {
        super.super.execute(); /* Does not work obviously */
        for (Command c : commands) {
            if (!c.getExecuted()) {
                c.execute();
            }
        }
    }
}

Basically I do want the safety of the Command's execute(), while I do not want the implementation of CompoundCommand's execute() for ExecutedCompoundCommand, but I do want to just rely on the add(), remove() and undo() operations of CompoundCommand.

As a student, working on a project with required javadoc and unit testing, it is really needed that there is as few code duplication as possible, as it only makes more work.

Upvotes: 4

Views: 170

Answers (4)

R K
R K

Reputation: 41

Use the delegate pattern for common functionality instead of inheritance. Or the template pattern if you want to use inheritance.

Upvotes: 1

Patricia Shanahan
Patricia Shanahan

Reputation: 26175

There are several ways of fixing this, and the best way depends on your intent. Here are a couple of suggestions:

Create a new class, CommandList, that supports the add(), remove(), and undo() operations.

CompoundCommand extends Command and has a CommandList.

ExecutedCompoundCommand extends Command and has a CommandList.

Another option is to create a new subclass of Command that supports the common operations and inherits Command's execute() method.

CompoundCommand would extend it and override just execute.

ExecutedCompoundCommand would also extend it, and so its super.execute() would be the Command execute().

Upvotes: 1

burna
burna

Reputation: 2962

I think it is a design flaw. You can apply the Template Method Pattern [GOF 325]

Intent: Define the skeleton of an algorithm in an operation, deffering some steps to subclasses. Template Method lets subclasses redefine certain steps of an algorithm without changing the algorithm's structure.

From Gang of Four Design Patterns

You want to be sure that certain steps are executed. So you would make a final template method execute() and delegate to the doExecute() method, which can add additional logic and needs to be implemented by the subclasses.

public final void execute() {
  importantOperation();
  runsAlways();
  doExecute();
}

public abstract void doExecute(); // Override in subclasses

Upvotes: 2

Jurgen Camilleri
Jurgen Camilleri

Reputation: 3589

Take a look here. Basically it explains why you should never need to do what you are looking to do.

As quoted from the link:

You shouldn't be able to bypass the parent class's behaviour. It makes sense to sometimes be able to bypass your own class's behaviour (particularly from within the same method) but not your parent's.

In the example shown in the link, the argument made is that the "middle" class, so to speak, is implementing some functionality or validity checks which will be bypassed by "skipping" a class in the hierarchy.

Read this small article for the benefits of encapsulation.

Upvotes: 0

Related Questions