2

In the context of sports scheduling, a scheduling rule violation (let's call it simply violation from now on) is produced when trying to allocate a match in an illegal timeslot. There is a wide variety of reasons of why this can happen and this is precisely the purpose of the scheduling violation object: a list of them is associated to a match when trying to allocate it, so we can tell the reason (or reasons) why this action failed.

To picture it more easily:

def allocate(matches, timeslots):
    for match in matches:
        for timeslot in timeslots:
            allocated, violation = allocate(match, timeslot)
            if allocated:
                break
            else:
                match.scheduling_violations.append(violation)

I am trying to achieve a proper design for a violation. I mentioned each violation gives a clue of why a match could not be allocated, which means we can categorize a violation. So we can start with that:

class SchedulingViolation:
    def __init__(self, match, category):
        self.match = match
        self.category = category

Now comes the first pothole: I want to come up with an straightforward way of defining such a category so we can easily pass in its value when instantiating a violation, for example:

SchedulingViolation(match, SchedulingViolation.TIMESLOT_OCCUPIED)
SchedulingViolation(match, SchedulingViolation.BLOCKED_TIME)
SchedulingViolation(match, SchedulingViolation.BLOCKED_DATE)

Enums came to mind, but these don't really exist natively in Python, although you can emulate them. All in all, I went for integers instead:

class SchedulingViolation:
    TIMESLOT_OCCUPIED, BLOCKED_TIME, BLOCKED_DATE = range(1, 4)

However, this has the difficulty of identifying the category: when displaying or printing a violation category a cryptic numeric number that means little is returned, so I need something to give these categories a meaningful representation when using __str__ or __repr__:

class SchedulingViolation:
    TIMESLOT_OCCUPIED, BLOCKED_TIME, BLOCKED_DATE = range(1, 4)
    categories = dict(zip((TIMESLOT_OCCUPIED, BLOCKED_TIME, BLOCKED_DATE), ('TIMESLOT_OCCUPIED', 'BLOCKED_TIME', 'BLOCKED_DATE')))

    def __str__(self):
        return 'SCHEDULING VIOLATION [{}]'.format(self.category)

However this looks rather ugly and redundant. I could add some logic in the __str__ method and associate a string to a particular integer code, but this also seems like a bad solution.

I think the problem lies in design, and this is why I am posting here. First question: how may I approach the category design problem?

There is a second pothole. There is a different categorizing attribute I would need to throw into the mix. I need to define two types of violations:

  • those produced by fixed constraints (i.e. a certain team cannot play at a particular time, and this will be true under any circumstances),
  • and those produced by loose constraints (these depend on the existence of other matches, i.e. a violation because of an already occupied timeslot; these constraints will not always be true, they can vary).

So I can just implement it as follows:

class SchedulingViolation:
    def __init__(self, match, category, fixed=True):
        self.match = match
        self.category = category
        self.fixed = fixed

However, these fixed and loose types are directly tied to the category: TIMESLOT_OCCUPIED will always be loose, and BLOCKED_TIME will always be fixed. This tempts me to branch out and make two different classes: FixedSchedulingViolation and LooseSchedulingViolation, to prevent having incoherent instances (i.e. TIMESLOT_OCCUPIED but fixed).

But I don't want to end up with an overly complex design. This makes me think again that I am developing a bad design to cover this scenario.

So yeah, I just want to check if approach is faulty as I suspect, and I would love some suggestions and ideas.

0

This design is inside out.

You are totally focused on what a SchedulingViolation is and are ignoring what it does.

What behavior do you expect to be different from one "category" to the next? Only difference I see you designing for is how __str__ behaves.

There needs to be some point to all of this. If I have to ask a SchedulingViolation if it's fixed and then run my own logic then it's not helping much on it's own. The logic needed to make that knowledge useful doesn't exist inside it when it should.

You haven't even defined what that logic would be. Why does anyone care if a SchedulingViolation is fixed or not? What difference does it make? Telling how you decide if it's fixed doesn't help.

Tell, don't ask teaches us that an object shouldn't expect others to ask it questions and do it's work for it. An object should just do what it's told. If you have to ask an object questions, make a decision, and do some work you've been fooled into doing the object's job for it.

This is a problem because when you do an objects job for it you end up entangled with it. It becomes very hard to make changes and understand what all needs to change. The object was supposed to create a clear boundary around a single responsibility but now that's scattered.

Figure out what work needs to be done because a SchedulingViolation is fixed, or not and what needs to be done based on category. Put all that logic into different SchedulingViolation classes and just tell them to do what you need done. Then you'll be using polymorphism. The nice thing about that is you don't have to know what you have. You just tell it and it does it. Tell dont ask is nifty that way.

Or you can insist on only putting ints and strings in the class and writing piles of code that ask for them to figure out which kind you have. Enjoy trying to maintain that as you add a new type of SchedulingViolation.

  • So you're suggesting I create an actual hierarchy of SchedulingViolation, then subclass it into FixedSchedulingViolation andLooseSchedulingViolation and then subclass for each different category? But the thing is having different classes for each type of scheduling violation wouldn't do much. Essentially these classes would do nothing, I am only using schedule violations as flags, as a way to store the reason why an attempt to allocate a match failed, and this task is the responsibility of another component, not SchedulingViolation's at all, it's merely a class to represent a state. – dabadaba Mar 23 '17 at 8:13
  • I know you are. I'm saying that's the problem. You've convinced yourself that a SchedulingViolation can't have any behavior. Just an int and a name. Oh and a bool. And whatever else you decide to add later and expose. You can do that. But don't call it object oriented, cause it's not. – candied_orange Mar 23 '17 at 10:19
  • So what can I do instead? I'm not convinced myself of nothing, I am not sure why the condescending tone, I am here to hear alternatives, I'm open to anything. Any suggestion is welcome. – dabadaba Mar 23 '17 at 11:09
  • Figure out what needs to be done differently because of the different kinds of SchedulingViolations. Before deciding on a design that stores information about whether the violation is fixed decide what needs to be done that cares if it's fixed or not. Decide what different behavior a BlockedTime time has from a BlockedDate and from a OccupiedTimeslot before you decide these all have to be different classes. That you can return different names from them isn't the best way to decide on a design. Find actual work for them to do. Otherwise this might as well just be a string. – candied_orange Mar 23 '17 at 11:28
  • The whole purpose of having these states is to know why a match cannot be allocated. With this information I'm aiming at being able to: a) letting the user know why the match could not be allocated and b) responding accordingly and try a different approach when allocating the match (for example, if we know a match could not be allocated because the timeslot is occupied, we can try freeing this slot by moving the occuppying match to another different timeslot). These tasks certainly don't look like they should be the responsibility of SchedulingViolation classes, they belong elsewhere. – dabadaba Mar 23 '17 at 11:52

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.