2

I have two ideas on how to proceed with an availability class but I'm not sure which one is better.

public class TimeRange {
private Timestamp startTime;
private Timestamp endTime;

public TimeRange() {
    //set defaults
}
// appropriate getters
//setters
public void setStartTime(Timestamp startTime) {
    this.startTime = startTime;
}

public void setEndTime(Timestamp endTime) {
    this.endTime = endTime;
}
// some code that validates the object is in a valid state.
}

First Idea:

public class WeeklyAvailabilities {
private TimeRange mondaySlot;
private TimeRange tuesdaySlot;
private TimeRange wednesdaySlot;
private TimeRange thursdaySlot;
private TimeRange fridaySlot;
private TimeRange saturdaySlot;
private TimeRange sundaySlot;

public WeeklyAvailabilities() {
   // set defaults
}
// some getters and such
public void setMondayStartTime(Timestamp start) {
   mondaySlot.setStartTime(start);
}

public void setMondayEndTime(Timestamp end) {
   mondaySlot.setEndTime(end);
}
// continue the pattern for all the slots
}

Second Idea:

public class WeeklyAvailabilities {
private TimeRange mondaySlot;
private TimeRange tuesdaySlot;
private TimeRange wednesdaySlot;
private TimeRange thursdaySlot;
private TimeRange fridaySlot;
private TimeRange saturdaySlot;
private TimeRange sundaySlot;

public WeeklyAvailabilities() {
   // set defaults
}
// some getters and such
public void setMondaySlot(TimeRange mondaySlot) {
   this.mondaySlot = mondaySlot;
}

// continue the pattern for all the slots
}

I'm conflicted due to the fact that the first idea doesn't require a new object to be created every time while the second idea would require something of the sort. Really just wondering if I could have some input on which one is better, or if there might be a third option that I'm completely overlooking.

4
  • 6
    Creating an extra object is no big deal, unless this is in an inner loop of a video game, which seems unlikely :-) . I would ** strongly** consider making TimeRange an immutable object, with a constructor taking two arguments. Otherwise, you always have to worry about the case where start is after stop.
    – user949300
    Dec 7, 2018 at 18:19
  • Thinking about Object allocation for something like this micro-optimization at it's finest. Use a tested library like JodaTime for your time modeling and provide all three methods. The methods which set just the start and the end can then create new time spans and let the time span representing class do the validation. Dec 7, 2018 at 23:38
  • Does the client code want to set the start and end separately or at once? Dec 9, 2018 at 12:47
  • The code should allow the user to be able to set start and end separately yes, once loaded from the database I'm thinking of allowing them to be able to say change their start time, their end time, or maybe even both.
    – Sahil Tara
    Dec 9, 2018 at 13:15

3 Answers 3

2

Both ideas have their pros and cons in many different aspects, with the amount of instance creation being one of the less important ones.

Most important is: what are the state changes of WeeklyAvailabilities that you want to support, making sure whatever setter someone calls in whatever situation, you always result in a consistent state (or throw an exception). And do you really foresee any changes to a WeeklyAvailabilities instance after its initialization?

Then the questions are:

How does a WeeklyAvailabilities instance reach its initial state? There seems to be a default setting in the no-args constructor. Will any WeeklyAvailabilities instance ever keep these values or will they typically set individual timeslots, making the no-args constructor rather worthless, and instead call for a full-args one (or application of some builder pattern).

Do you think someone will want change the start or end times individually, not pair-wise? Only then should you offer individual setters.

To sum it up, my wild guess is that you might be better off with a seven-TimeRange constructor and no setter at all.

3
  • 2
    +1 for the "no setter at all" - the question reads like "which of these two options is less wrong?" Default to immutable structures if at all possible, because they are just so much more understandable. Dec 8, 2018 at 16:18
  • To answer some of the questions it is possible for timeslots to have their default values, but not all of them have to keep them. So maybe the builder would be a better choice here, however I just have a concern for the following situation: lets say I load a weeklyavailabilities class from the database, and lets say I want to change just the start time or just the end time of monday for example. Would a builder that takes the old weeklyavailabilities and allows changes to it via changeStartTime() changeEndTime() be more appropriate?
    – Sahil Tara
    Dec 9, 2018 at 13:24
  • 1
    I see you expect individual changes of e.g. the tuesday end time long after initial creation, so the individual setters are a good choice matching the state-change behavior that you need. Dec 9, 2018 at 18:15
2

Without further information, your second idea looks better, because it simplifies the interactions with TimeRange (decreasing coupling), and allows you to delegate to that class the consistency checks.

Also consider this refactoring, which reduces code replication and improves maintainability:

import java.util.*;

public class WeeklyAvailabilities {
    public static enum WeekDay { MONDAY, TUESDAY, ... };

    private Map<WeekDay,Integer> slots = new EnumMap<>(WeekDay.class);

    public WeeklyAvailabilities() {
    // set defaults
    }

    // some getters and such
    public void setSlot(WeekDay day, Integer slot) {
        slots.put(day, slot);
    }

// continue the pattern for all the slots
}
1
  • Thanks for the idea, I used this to refactor my code a bit, and must I say my code looks a lot cleaner now thanks to this. I completely forgot enum maps were a thing!
    – Sahil Tara
    Dec 10, 2018 at 11:35
1

Your first option defeats the purpose of the TimeRange class, you might as well have 14 TimeStamps in WeeklyAvailabilities directly. Which would not necessarily be that bad if you want to allow incremental population of WeeklyAvailabilities but you would lose the time slot concept, getting a flatter model.

To prevent clutter and a needless amount of methods I would use a WeekDay enum as a second argument in setSlot. And change the name TimeRange to TimeSlot because you now use two different words for the same thing.

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.