2

I am really reconsidering my current class hierarchy for timeslots (in a schedule). I wanted to make them as flexible as possible so it would cover many possibilities of what we could understand as a timeslot, so I ended up with this design:

All timeslots share a chronological/hierarchical order so we're able to define comparing functions. It provides the most basic means of determining if a timeslot comes before or after another.

public interface Timeslot extends Comparable<Timeslot> {
   int getChronologicalOrder();
} 

This is the "base" timeslot, it just has a chronological order integer.

public class AbstractTimeslot extends Entity implements Timeslot {
    protected final int chronologicalOrder;
}

This is for timeslots that define a range, that is, the span of the timeslot in the schedule, for example 10 minutes, 1 hours, 3 days, etc... To represent this we use a TemporalAmount member. However, the starting point of the timeslot is unknown or irrelevant.

public abstract class RangedTimeslot extends AbstractTimeslot {
    protected final TemporalAmount duration;
}

Then we have this timeslot which is like the previous one but it does define a definite starting point using the Java 8 interface TemporalAccessor. This way we can say when a timeslot starts, like a particular day or hour with LocalDate, LocalTime, DayOfWeek, etc.

public class DefiniteTimeslot extends RangedTimeslot {
    protected final TemporalAccessor start;
}

Finally there's this timeslot that doesn't define a duration because it is unknown or irrelevant, but we know the starting time. It is exactly like a RangedTimeslot but I wanted to keep this last one as an abstract class in case we needed further hierarchy expansion.

public class UndefiniteTimeslot extends RangedTimeslot {}

Visually, this is how the hierarchy looks like:

enter image description here

Now I am wondering if this is making things harder than they actually are. The first big wall I am facing with this design is that now I need to serialize and deserialize as JSON many of my classes, like Tournament and Schedule, and they include these timeslot classes. This kind of polymorphism makes it kind of a hell to deal with serialization and deserialization (I am using Jackson), so I just started wondering if this is a good design and if it'd be a good idea to "simplify" it. I'd still have to face the polymorphism problem of TemporalAmount and TemporalAccessor but that's kind of inevitable.

I think the other way of thinking this scenario is cutting the whole tree and be left with just a Timeslot class with Optional members so every situation where we want undefined starting times and/or undefined timeslot span are covered. We would end up with a class more or like the one that follows:

public class Timeslot extends Comparable<Timeslot> {
    private int chronologicalOrder;
    private final Optional<TemporalAccessor> start;
    private final Optional<TemporalAmount> duration;

    public Timeslot(int c) {
        chronologicalOrder = c;
        start = Optional.empty();
        duration = Optional.empty();
    }

    public Timeslot(int c, TemporalAccessor s) {
        chronologicalOrder = c;
        start = s;
        duration = Optional.empty();
    }

    public Timeslot(int c, TemporalAmount d) {
        chronologicalOrder = c;
        start = Optional.empty();
        duration = d;
    }

    public Timeslot(int c, TemporalAccessor s, TemporalAmount d) {
        chronologicalOrder = c;
        start = s;
        duration = d;
    }
}

What do you think is the best design? Should I dump my current hierarchy and take the last solution? Would it make testing the class easier? (I already have tests for the current design)

14
  • 3
    I'm wondering what the ordering of "3 days" and "Next Tuesday" is
    – Caleth
    Jun 10, 2016 at 10:02
  • What is your problem with jackson here ? I have no problem doing this with jackson.
    – Walfrat
    Jun 10, 2016 at 10:08
  • @Caleth although an odd situation, that's why the chronological order exists, it determines the order.
    – dabadaba
    Jun 10, 2016 at 10:54
  • @Walfrat the problem is not Jackson specific, but serialization/deserialization rather because there are subclasses. However as I mentioned the TemporalAccessor and TemporalAmount serialization/deserialization is a bigger issue that the hierarchy itself, tbh.
    – dabadaba
    Jun 10, 2016 at 10:56
  • @dabadaba I saw that, but how do you assign chronological order for the two examples I gave? And if the answer is "We write some helper function on TemporalAccessor s, TemporalAmount d", how do you expect people using this type to internalise the rules of that function?
    – Caleth
    Jun 10, 2016 at 10:59

2 Answers 2

1

Your question is "design A or design B?" and my answer is neither. Actually, I'm going to say that aside from the interface, your Timeslot class doesn't really matter and that both of these designs are a result of coming at this from the wrong direction.

At the risk of overextending an analogy, consider the question: 'what is a slot?' A slot is a type of hole. And we all know that the question "how many holes does it take to fill X?" is nonsensical.

Kidding (sort of) aside, you really shouldn't have a public constructor on your timeslot class. Timeslots are really an attribute of a calendar. Trying to use them in isolation or trying to build the calendar from (heterogeneous) timeslots is problematic at best. If I were designing something in this area, I would make the timeslots inner classes or even anonymous inner classes of a schedule class. Here's an example of how that might look from an API perspective (this could be very different depending on what you need to achieve e.g. are timeslots of fixes size or of varying size):

interface Schedule
{
    List<Timeslot> getAvailable()

    boolean reserve(Timeslot timeslot)
}

The only place timeslots would be created is within the Schedule object. This allows the timeslots to be created in a consistent and reliable way. For example, if the timeslots can be created and passed in, it's harder to avoid getting the same slot created twice or perhaps you might get overlapping slots.

Once you get to this point, you should realize that a timeslot is just a porthole view of the whole schedule and the hierarchy has no need to exist because the structure is based on how the schedule is managed.

0

Jackons is used to serialize "dumb" POJO, there you have object with intelligence (TemporalAccessor), this is not compatible.

You can :

  • move it away on some service layer
  • use DTO layer to convert your business entity to a POJO
  • Remove empty contructor on higher classes, having one protected constructor which will take the TemporalAccessor implementation in parameter
  • Define somewhat a custom serializer/deserializer that will instantiate the right TemporalAccessor this may be tricky and not necessary

I'll develop the 3rd choice, this would give something like :

public abstract class RangedTimeslot extends AbstractTimeslot {
    protected final TemporalAmount duration; 
    protected RangedTimeslot(TemporalAmount duration){
       this.duration = duration;
    }
}

public abstract class DefiniteTimeslot extends RangedTimeslot {
    protected final TemporalAccessor start;
     protected DefiniteTimeslot (TemporalAmount duration, TemporalAccessor start){
       super(duration);
       this.start = start;
    }
}

public class MyTimeSlot extends DefiniteTimeslot{
       public MyTimeSlot(){
            // or cached static reference 
            super(new TemporalAmountImpl(), new TemporalAccessorImpl());
       }
}

Another way using generics

public abstract class RangedTimeslot<T extends TemporalAmount> extends AbstractTimeslot{
    protected RangedTimeslot(T duration){...}
}

public abstract class DefiniteTimeslot<T extends TemporalAmount, U extends TemporalAccessor> extends RangedTimeslot<T>{
      protected DefiniteTimeslot (T duration, U start){}
}

public class MyTimeSlot extends DefiniteTimeslot<TemporalAmountImpl, TemporalAccessorImpl>{
       public MyTimeSlot(){
            // or cached static reference 
            super(new TemporalAmountImpl(), new TemporalAccessorImpl());
       }
}
9
  • This wasn't a Jackson question. It's a question to decide if I should stick to my hierarchy or have a unique Timeslot class with Optional members. Obviously I am not dealing with POJOs, I am implementing my own custom serializers and deserializers, but then again, this is not the scope of my question.
    – dabadaba
    Jun 10, 2016 at 11:38
  • @dabadaba So the code that i did is not about design ? It has nothing relevant with Jackson yet is compatible. I suggested you others design solution from the Optional.. Is your question is a A or B solution and not C ? If then my bad because i'm suggesting a C/D/... design option which are improvment of A. Compatible with Jackson, yet not so relevant with it. Because i don't tihnk Optional is really a good fit here.
    – Walfrat
    Jun 10, 2016 at 11:52
  • Ok I see your point now, then this is a valid answer. However, I fail to understand your design. So you're suggesting all my timeslot classes are to be abstract? And then what? The user should define their own classes that extend from these abstract ones?
    – dabadaba
    Jun 10, 2016 at 12:04
  • @dabadaba Yes, this is a common pattern with generics. I edited my post and add some code, the same solution, with generics this time. This is supposing that each type will always support only one implementation of those. If not this won't fit.
    – Walfrat
    Jun 10, 2016 at 12:12
  • so if the user must provide the implementation, must he also provide the custom serializer and deserializer I guess?
    – dabadaba
    Jun 10, 2016 at 12:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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