0

I'm coding in Python, but the question seems independent of programming language.

I have a class that represents some system check:

class Check:
   @abstractmethod
   def run()
     """ You have to define your own run(). As a result, it must set self._ok. """
      ...

   @property
   def is_ok():
      return self._is_ok

Then we have a set of checks by subclassing Check class, and they're used in the following way (simplified):

class Checker:
    checks = [check1, check2...]

    def __call__(self):
        for check in self.checks:
            if not check.is_ok:
                alarm()

The question is: Is it fine to oblige subclass to set some protected object attributes?

Jonathan Eunice
  • 9,710
  • 1
  • 31
  • 42
  • "independent of programming language" -- in this case, it's likely a duplicate of [Are trivial protected getters blatant overkill?](http://programmers.stackexchange.com/questions/215450/are-trivial-protected-getters-blatant-overkill) – gnat Nov 05 '14 at 12:08

3 Answers3

2

Why wouldn't it be okay to require subclasses to do work? The parent is abstract, after all.

The more important question is: why is run() supposed to signal success via an out-of-bound mechanism (setting a global variable) rather than a return value? There are sometimes reasons for doing this (e.g. because it's an extremely entrenched convention, like errno in C), but usually it's a bad sign because of issues with concurrency, maintainability etc.

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
  • I'd like Check to have state. Returning value is an obvious option, but I don't like it in such particular case. – Eugene Koval Nov 05 '14 at 11:35
  • 1
    My concern is: Isn't it too much to explain to the child class, and isn't it too hard to verify if child class fits this contract? If we refactor it to return result, it's easier. Just return True/False and that's it. – Eugene Koval Nov 05 '14 at 12:20
2

To provide an alternative to letting the value be set by the subclass you can instead have them provide a doRun that returns true/false to signify success and the run assigns that result to self._ok, this also allows more checks like catching exceptions or ensuring it only runs once:

class Check:
   @abstractmethod
   def doRun()
     """ You have to define your own doRun(). As a result, it must return whether it was successful. """
      ...

   def run()
     if not is_ok """ runs once, remove if not needed """
         self._ok = doRun()

   @property
   def is_ok():
      return self._is_ok
ratchet freak
  • 25,706
  • 2
  • 62
  • 97
  • Thanks! Yes, I understand that this can be refactored in a number of ways, but I'd like to know whether my approach violates some design rules or not. – Eugene Koval Nov 05 '14 at 12:01
0

One commonly (albeit not universally) accepted principle of object design is that an object should be immutable unless this makes the design substantially more complex. Immutable objects are easier to reason about, can be used in a multithreaded environment more easily and are generally less prone to bugs.

From the code you have shown us, there is no reason I can see why you need mutable objects, so I would use immutable ones.

Jules
  • 17,614
  • 2
  • 33
  • 63
  • Is the rule "keep objects immutable" related to data-driven design? I always thought that objects are data+code by definition, so they can (or should) have state. – Eugene Koval Nov 06 '14 at 14:11