4

I have a couple of schedule value classes that represent the value a schedule holds for a specific slot.

First, there's the ScheduleValue class for schedules with two dimensions, where the first one (rows) is the players and the second (columns) the timeslots (when the players play each match).

I represented the different possible values with public static ints, and this is what you would use to instantiate a new ScheduleValue:

public class ScheduleValue {
    public static int OCCUPIED = 1;
    public static int FREE = 2;
    public static int UNAVAILABLE = 3;
    public static int BREAK = 4;
    public static int LIMITED = 5;
    public static int NOT_IN_DOMAIN = 6;

    private int value;

    public ScheduleValue(int val) {
        value = val;
    }
}

However, there's a special value, OCCUPIED, that demands an integer to be passed, so the localization (court) where the match takes places is represented by the schedule value. So I added another constructor and I was forced to add controls to prevent wrong states from happening:

public ScheduleValue(int val) {
    if (!(val >= OCCUPIED && val <= NOT_IN_DOMAIN))
        throw new IllegalArgumentException("Illegal value (" + val + ")");

    if (val == OCCUPIED)
        throw new IllegalArgumentException("A localization must be specified if the schedule value is OCCUPIED");

    value = val;
}

public ScheduleValue(int val, int l) {
    if (!(val >= OCCUPIED && val <= NOT_IN_DOMAIN))
        throw new IllegalArgumentException("Illegal value (" + val + ")");

    if (val != OCCUPIED)
        throw new IllegalArgumentException("Only schedule values of OCCUPIED can specify a localization");

    value = val;
    localization = l;
}

I really don't like the way I have done this. Both having to throw exceptions and the way the values are being checked (using relational operations instead of checking if they're contained in a list of permitted values). How could I redesign it? Should I create a ScheduleValueOccupied class solely for the purpose of holding the value of the localization?

Then, I also have a LocalizationScheduleValue that was made for another type of schedules where the first dimension is the localizations where the matches take place, instead of the players (so the matrix is courts by timeslots instead of players by timeslots). And although it looks pretty much like ScheduleValue, it doesn't even extend from it:

public class LocalizationScheduleValue {
    public static int OCCUPIED = 1;
    public static int FREE = 2;
    public static int UNAVAILABLE = 3;
    public static int LIMITED = 4;
    public static int CONTINUATION = 5;
    private int value;
    private List<Integer> playersIndices;

    public LocalizationScheduleValue(int val) {
        if (!(val >= OCCUPIED && val <= CONTINUATION))
            throw new IllegalArgumentException("Illegal value (" + val + ")");

        if (val == OCCUPIED)
            throw new IllegalStateException("A match must be specified if the schedule value is OCCUPIED");

        value = val;
    }

    public LocalizationScheduleValue(int val, List<Integer> indices) {
        if (!(val >= OCCUPIED && val <= CONTINUATION))
            throw new IllegalArgumentException("Illegal value (" + val + ")");

        if (val != OCCUPIED)
            throw new IllegalStateException("Only schedule values of OCCUPIED can specify a match taking place");

        value = val;
        playersIndices = indices;
    }
}

I would like to change this class so it relates to ScheduleValue, but I don't know how for the reasons I am going to mention.

My main concern is about the way I represent values internally, should I stick to integers or should I change to enums?

Then, about creating a new hierarchy. If I end up having a ScheduleValue, ScheduleValueOccupied, LocalizationScheduleValue and LocalizationScheduleValueOccupied, how can I represent all the possible values? If I use ints I would have to keep track of the numbers I used in the parent, otherwise there would be a clash; example:

public class ScheduleValue {
    public static int OCCUPIED = 1;
    // ...
}

public class LocalizationScheduleValue extends ScheduleValue {
    public static int OCCUPIED = 1; // clash with parent here
    // ...
}

While if I use enums, how can I only inherit part of them and addionaly throw in some more values? Example:

public class ScheduleValue {
    protected enum Value { OCCUPIED, FREE, UNAVAILABLE, BREAK, LIMITED, NOT_IN_DOMAIN };
    // ...
}

public class LocalizationScheduleValue extends ScheduleValue {
    // I would only want OCCUPIED, FREE and UNAVAILABLE, but not the rest
    // and I would need to add to the possible values LIMITED and CONTINUATION
}

I am pretty confused about how to restructure this whole thing, especially the issue regarding the internal representation and its inheritance. There's also the name redundancy, how could I name the enum?

Maybe I am overthinking this and the solutions is simpler than it seems to me. I hope it's one of this times you say how could I not see this?

Any suggestions?

4
  • Does it make sense to not use an enum or an int but an actual class hierarchy to represent this value? This way you can subset the possible argument types for your LocalizationScheduleValue by accepting only those that inherit a certain interface/abstract class. It might also help reducing switch statements by moving the routines to the Value classes or using the Visitor pattern.
    – Teimpz
    May 15, 2016 at 18:10
  • @Teimpz I would have no problem to make another class for that purpose, but could you go further? I did not understand your idea.
    – dabadaba
    May 15, 2016 at 18:17
  • I will post a more detailed explaination as an answer and include some code snippets then when I get home. I'm on my phone right now.
    – Teimpz
    May 15, 2016 at 18:22
  • Polymorphism as set out below, plus take a look at the command pattern. Mar 12, 2017 at 18:08

1 Answer 1

1

You could opt to use a class instead of the int or enum in your examples. (I do not program in Java so forgive me if I get the syntax wrong)

public interface Value{
}
public interface LocalizationValue implements Value{
}
public class Occupied implements LocalizationValue{
}
public class Free implements LocalizationValue{
}
public class Limited implements Value{
}
//...

this way, you can make sure the contructors can only accept the correct types:

public ScheduleValue( Value val ){ 
     //...
}
public LocalizationScheduleValue( LocalizationValue val ){
     //...
}

Of course you should not just leave this classes empty. They are real classes now and should start taking some responsibility. For example lets state that there are several if/else statements like this in the codebase:

public void doSomething( Value val ){
    if( val == OCCUPIED ){
         doSomethingOccupied();
    else if( val == FREE ){
         soSomethingFree();
    }
    //...
}

These switches are no longer needed now because you can define doSomething() in the Value interface and implement it in the concrete classes.

public interface Value{
    public void doSomething();
}
public class Occupied implements Value{
    public void doSomething(){
        //...
    }
}
public class Free implements Value{
    public void doSomething(){
        //...
    }
}
//...

public void doSomething( Value val ){
    val.doSomething();
}

When it makes no sense to implement the doSomething method in the Value classes, it can be extracted by using the visitor pattern. I am not going to type out code snippets for this but you should look up the Visitor pattern if you don't know it. This is safer that the long if/else switch because if you would add another Value type, the compiler will complain untill you've implemented the correct functions.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.