Update documentation

python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.py

@@ -1,48 +0,0 @@
-#Superclass __init__ calls subclass method
-
-class Super(object):
-
-    def __init__(self, arg):
-        self._state = "Not OK"
-        self.set_up(arg)
-        self._state = "OK"
-
-    def set_up(self, arg):
-        "Do some set up"
-
-class Sub(Super):
-
-    def __init__(self, arg):
-        Super.__init__(self, arg)
-        self.important_state = "OK"
-
-    def set_up(self, arg):
-        Super.set_up(self, arg)
-        "Do some more set up" # Dangerous as self._state is "Not OK"
-
-
-#Improved version with inheritance:
-
-class Super(object):
-
-    def __init__(self, arg):
-        self._state = "Not OK"
-        self.super_set_up(arg)
-        self._state = "OK"
-
-    def super_set_up(self, arg):
-        "Do some set up"
-
-
-class Sub(Super):
-
-    def __init__(self, arg):
-        Super.__init__(self, arg)
-        self.sub_set_up(self, arg)
-        self.important_state = "OK"
-
-
-    def sub_set_up(self, arg):
-        "Do some more set up"
-
-

python/ql/src/Classes/InitCallsSubclass/InitCallsSubclassMethod.qhelp

@@ -4,37 +4,36 @@
 <qhelp>
 <overview>
 <p>
-When an instance of a class is initialized, the super-class state should be
-fully initialized before it becomes visible to the subclass.
-Calling methods of the subclass in the superclass' <code>__init__</code>
-method violates this important invariant.
+When initializing an instance of the class in the class' <code>__init__</code> method, calls tha are made using the instance may receive an instance of the class that is not 
+yet fully initialized. When a method called in an initializer is overridden in a subclass, the subclass method receives the instance
+in a potentially unexpected state, which may lead to runtime errors from accessing uninitialized fields, and generally makes the code
+more difficult to maintain. 
 </p>
 
 </overview>
 <recommendation>
 
-<p>Do not use methods that are subclassed in the construction of an object.
-For simpler cases move the initialization into the superclass' <code>__init__</code> method,
-preventing it being overridden. Additional initialization of subclass should
-be done in the <code>__init__</code> method of the subclass.
-For more complex cases, it is advisable to use a static method or function to manage
-object creation.
+<p>If possible, refactor the initializer method such that initialization is complete before calling any overridden methods. 
+For helper methods used as part of initialization, avoid overriding them, and instead call any additional logic required 
+in the subclass' <code>__init__</code> method.
+</p><p>
+If calling an overridden method is required, consider marking it as an internal method (by using an <code>_</code> prefix) to 
+discourage external users of the library from overriding it and observing partially initialized state. 
 </p>
 
-<p>Alternatively, avoid inheritance altogether using composition instead.</p>
-
 </recommendation>
 <example>
+<p>In the following case, the `__init__` method of `Super` calls the `set_up` method that is overriden by `Sub`. 
+This results in `Sun.set_up` being called with a partially initialized instance of `Super` which may be unexpected. 
 <sample src="InitCallsSubclassMethod.py" />
-
+<p>In the following case, the initialization methods are separate between the superclass and the subclass.
 </example>
 <references>
 
 
 <li>CERT Secure Coding: <a href="https://www.securecoding.cert.org/confluence/display/java/MET05-J.+Ensure+that+constructors+do+not+call+overridable+methods">
-Rule MET05-J</a>. Although this is a Java rule it applies to most object-oriented languages.</li>
-<li>Python Standard Library: <a href="http://docs.python.org/library/functions.html#staticmethod">Static methods</a>.</li>
-<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Composition_over_inheritance">Composition over inheritance</a>.</li>
+Rule MET05-J</a>. Reference discusses Java but is applicable to object oriented programming in many languages.</li>
+<li>StackOverflow: <a href="https://stackoverflow.com/questions/3404301/whats-wrong-with-overridable-method-calls-in-constructors">Overridable method calls in constructors<a>.</li>
 
 
 

python/ql/src/Classes/InitCallsSubclass/examples/InitCallsSubclassMethodBad.py

@@ -0,0 +1,23 @@
+class Super(object):
+
+    def __init__(self, arg):
+        self._state = "Not OK"
+        self.set_up(arg) # BAD: This method is overridden, so `Sub.set_up` receives a partially initialized instance.
+        self._state = "OK"
+
+    def set_up(self, arg):
+        "Do some setup"
+        self.a = 2
+
+class Sub(Super):
+
+    def __init__(self, arg):
+        super().__init__(arg)
+        self.important_state = "OK"
+
+    def set_up(self, arg):
+        super().set_up(arg)
+        "Do some more setup"
+        # BAD: at this point `self._state` is set to `"Not OK"`, and `self.important_state` is not initialized.
+        if self._state == "OK":
+            self.b = self.a + 2

python/ql/src/Classes/InitCallsSubclass/examples/InitCallsSubclassMethodGood.py

@@ -0,0 +1,24 @@
+class Super(object):
+
+    def __init__(self, arg):
+        self._state = "Not OK"
+        self.super_set_up(arg) # GOOD: This isn't overriden. Instead, additional setup the subclass needs is called by the subclass' `__init__ method.`
+        self._state = "OK"
+
+    def super_set_up(self, arg):
+        "Do some setup"
+        self.a = 2
+
+
+class Sub(Super):
+
+    def __init__(self, arg):
+        super().__init__(arg)
+        self.sub_set_up(self, arg)
+        self.important_state = "OK"
+
+
+    def sub_set_up(self, arg):
+        "Do some more setup"
+        if self._state == "OK":
+            self.b = self.a + 2