3

I was assigned to design a piece of software (in java) with projects and jobs where jobs each have a status. As we learned about the GoF-patterns, the State pattern seemed like an obvious choice to me, but now I'm having some troubles with the implementation.

In my current design I have a Job-class to represent the job with a certain state and a Project-class that has a list of jobs. To represent the four possible states (AVAILABLE - UNAVAILABLE - FINISHED - FAILED), I made an enum State with the state dependent methods of job implemented in the enum.

The issues with the state-pattern I'm having are the following:

  1. In Project, I would like to have a method that returns all AVAILABLE jobs of the project (and maybe even one that returns all jobs that are either FINISHED or FAILED), but the state pattern doesn't mention on how to do this. We also got to hear that having methods as public boolean isAvailable() in Job or State should not be used when using the state pattern (because this is exactly what one wants to avoid when using this pattern).
  2. There is need for a method that changes the state from an AVAILABLE job to either FINISHED or FAILED. The problem with this is that I'm not really sure on how to implement this when using the State pattern.

I have some solutions in mind, but I would like to know whether there are other/better solutions to implement the state pattern well.

  1. For the first problem, I would implement a public Collection<Job> filter(Collection<Job>) method in State that would return the subset of the given set with only those jobs that are of the current state. The getAvailableJobs() method in Project would be like below. The disadvantage of this approach for me is that coupling grows when Project needs to know about State as well.
  2. For the second problem I could either write a update(State newState) method where I could check whether newState is either finished or failed or I could write a method fail() and a method finish() (both in State of course). The problem with the second seems to be the fact that some of the state logic gets back in the Job, but I'm not sure whether the update method is the best method either.

I hope I have been clear enough and that someone can help me understand why these solutions would be good enough or bad and what possible alternatives might be.

Thanks in advance.


Some extra code to make some more things clear:

public class Project {

    private final Set<Job> jobs;
    // constructors, getters, setters, ...
    // for updating, I need to be able to present only the available tasks
    public Collection<Job> getAvailableJobs() {
        return Status.AVAILABLE.filter(getJobs());
    }

}

public class Job {
    private final State state;
    // Constructors, getters, setters, ...
    // update either to finished or failed 
    // (Timespan is nothing more than a pair of times)
    public void update(Timespan span, State newState) {
        getState().update(this, span, newState);
    }

}

public enum State {
    AVAILABLE {
        @Override
        public void update(Job job, Timespan span, State newState) {
            if(newState == AVAILABLE || newState == UNAVAILABLE)
                throw new IllegalArgumentException();
            job.setSpan(span);
            job.setState(newState);
        }
    },
    UNAVAILABLE, FINISHED, FAILED;

    public void update(Job job, State newState) {
        throw new IllegalStateException();
    }

    public Collection<Job> filter(Collection<Job> jobs) {
        Collection<Job> result = new HashSet<>();

        for(Job j : jobs)
            if(j.getStatus() == this)
                result.add(j);

        return result;
    }
}
jaco0646
  • 15,303
  • 7
  • 59
  • 83
  • Does your state indicate a different behaviour in class Job? What is the relation between Job and taks that is relevant for your second question? – swinkler May 04 '15 at 20:43
  • @swinkler I mixed up two terms. Jobs and tasks have no relation. The edit should have fixed that. – Mr Tsjolder from codidact May 07 '15 at 06:27
  • Is AVAILABLE to FINISHED or FAILED the ONLY state transition? You did not mention any other. – swinkler May 07 '15 at 07:05
  • @swinkler There are more, but I considered them not really relevant for my problem. A task can go for instance from `UNAVAILABLE` to `AVAILABLE` when certain conditions are met. The only reason I mentioned these transitions is because the UI should provide a way to assign a certain state to `Job` – Mr Tsjolder from codidact May 07 '15 at 09:06

4 Answers4

1

Well, I had some time, and I do enjoy implementing patterns, so I tried to implement your concept here. Seems like the challenge is about how to maintain lists of Jobs sorted by Job state, when the state is hidden in Job. Below is how I would do it, if you can't just have a field in Job that's public to use.

The Job would hold a reference to the Project object, a reference to it's state object, and defines the interface IJob (this is written in C#). Each state then ensures state consistency with its environment by calling methods in Project to notify it of changes. You will have to create an interface on Project that makes sense. I put in a structure here, but you'll have to define your own, whatever makes sense.

public class Project {

    private List<Job> listOfJobs;
    private List<Job> listOfAvailJobs;
    private List<Job> listOfNotAvailJobs;

    // Factory Method
    public Job CreateJob() {}

    public void JobIsAvailable(Job job) {
        listOfAvailJobs.Add(job);   
    }
    public void JobIsNotAvailable(Job job) {
        listOfNotAvailJobs.Add(job);
    }

}


public interface IJob {
    void Run();
    void Abort();
    void Delete();
}


public class Job : IJob {

    public Project project;
    protected JobState currentState;

    public Job(Project project) {
        this.project = project;
        currentState = new JobStateInit();
        project.JobIsAvailable(this);
    }

    // IJob Interface
    public void Run();
    public void Abort();
    public void Delete();

}

public abstract class JobState : IJob {

    protected Job job;

    public JobState(Job job) {
        this.Job = job;
    }

    // IJob Interface
    public void Run();
    public void Abort();
    public void Delete();

}

public class JobStateInit : JobState {
    // IJob Interface
    public void Run();
    public void Abort();
    public void Delete();
}

public class JobStateAvail : JobState {

    // IJob Interface
    public void Run() {
        this.job.project.JobIsNotAvailable(this.job);
    }
    public void Abort();
    public void Delete();
}

public class JobStateFailed : JobState {
    // IJob Interface
    public void Run() {
        throw new InvalidOperationException;
    }

    public void Abort() {}
    public void Delete() {}
}
gdbj
  • 16,102
  • 5
  • 35
  • 47
  • I'm not truly convinced this loosens coupling as this introduces the project job relation to be bi-directional. Nevertheless I think it's quite a nice solution for which I would like to thank you. – Mr Tsjolder from codidact May 05 '15 at 19:44
  • I guess the real de-coupling (when using the filter) could be done with just a `Filter` interface that has a method `public Collection extends Job> filter(Collection extends Job>)` – Mr Tsjolder from codidact May 07 '15 at 06:32
1

@Question 1: As long as only a project has to filter its jobs the filter method should be part of the Project class. You could provide a generic method that filters by a given state and also your concrete one (if this is used often why not taking it as part of your API):

class Project {
  private Set<Job> jobs;

  public Set<Job> filterJobs(JobState state) {
    return jobs.stream().filter(j -> j.getState() == state).collect(Collectors.toSet());
  }

  public Set<Job> availableJobs() {
    return filterJobs(JobState.AVAILABLE);
  }

}

Both work fine and are transparent for the user:

project.availableJobs();
project.filterJobs(JobState.<your state>);

@Question 2: Still thinking "how much state" is in it. Taking the description there are only state transitions from AVAILABLE (2) and only one method related to it. From my experience and understanding, Design Patterns should be applied if necessary, not from scratch. Your solution is quite fine the way it is. But considering the few requirements described here, it could be much simpler without the state pattern. (=YAGNI)

swinkler
  • 1,703
  • 10
  • 20
0

To avoid coupling between Job and State, you could move your "filter" method to a separate class, and you could further decrease the coupling by having Job implement an interface that only has a method for returning the state.

At this point, you've created a functional interface. If you're using Java 8, you could just as easily create a Lambda expression to do the same thing. Of course, that may be getting a bit ahead of where you are, but hopefully you get the idea.

Don
  • 391
  • 2
  • 9
  • That's indeed an idea, but I'm afraid cohesion would decrease with this solution (you can always have a separate class for every method). I would just like to keep the existence of `State` hidden for all other classes but `Task`. – Mr Tsjolder from codidact May 05 '15 at 07:33
0

You reference the State pattern in your question, but most of the details around your question have nothing to do with the State pattern.

The State pattern would only be limited to the interface design for Job, and handling of the actions that can be performed on Job. Each state that Job is in must then provide some kind of implementation of this interface. ie: run(), quit(), abort(). These methods would be called on Job and the implementation would alter depending on which state Job is in (ie: AVAILABLE, FINISHED, or FAILED).

To achieve this, you declare a class Job and then an abstract class JobState, with (concrete) subclasses JobStateAvail, JobStateFailed, etc. Each subclass would implement the Job interface. Any calls to Job from a client would then be delegated to the current JobState subclass for proper handling.

The relationship between Job and Project is a bit unclear to me. However this relationship is not related to the State pattern, so this is why you may be confused. I would need more information on the purpose of the Project class, and how Jobs are created. To achieve lists of Jobs according to state, what you could do is have the Job add itself to a defined list in Project. Perhaps this is what you are intending?

gdbj
  • 16,102
  • 5
  • 35
  • 47
  • I'm sorry, but I think they do have something to do with the State pattern. After all the state pattern is applied to increase cohesion, by getting all state transitions in one place. When you get methods like `isAvailable()` in there, the logic might escape from `State` to any other class using this method. The state pattern as such is described without taking into account that other classes (than the classes with the state) might be interested in the state as well. In my example, this is the case and I would like to know what possible alternatives might be to handle this. (PS: see edits) – Mr Tsjolder from codidact May 05 '15 at 07:27