Is this type of attribute value/function checking, a code smell in Python?
<p><em>note</em>: This is <a href="http://codereview.stackexchange.com/questions/142983/is-this-type-of-attribute-value-function-checking-a-code-smell-in-python">crossposted</a> on CodeReview, as per recommendation</p>

<p><strong>Premise</strong>: I have a class hierarchy (Python), where <code>tidy</code> is one of the methods. It removes nodes that are of type <em>ASTIgnore</em>, and rebinds the children of that node to its parent.</p>

<p>The target node cannot delete itself, and doesn't <em>see</em> its parent (for rebinding). Thus, the deletion of the target (of type ASTIgnore) would happen at its parent, where the parent <em>checks the type</em> of its children.</p>

<p><strong>Question</strong>: How would this need to be implemented to reduce the code smell?</p>

<p>Which among these approaches is least bad, or are there other ones (see bottom)?</p>



<pre><code># A)
if child.nodetype == "ASTIgnore":

# B)
if child.isIgnored():

# C)
if child.isIgnoreType:

# D)
if isinstance(child, ASTIgnore):
</code></pre>

<p>Where, the classes and <code>tidy</code> looks like the below. I'll remove the redundancy, based on the cleanest implementation.</p>



<pre><code>class ASTNode(object):
    def __init__(self):
        self.nodetype = self.__class__.__name__
        self.isIgnoreType = False

    def isIgnored(self):
        return False

    def tidy(self):
        # Removes "Ignore" type/attribute nodes while maintaining hierarchy
        if self.children:
            for child in self.children:
                child.tidy()

            for i, child in reversed(list(enumerate(self.children))):
                #--------- Is this Bad? ----------
                if child.nodetype == "ASTIgnore":
                #------ --------------------------
                    if not child.children:
                        # leaf node deletion
                        self.children.pop(i)
                    else:
                        # target node deletion + hierarchy correction
                        grandkids = child.children
                        self.children[i:i+1] = grandkids


class ASTIgnore(ASTNode):
    def __init__(self):
        ASTNode.__init__()
        self.isIgnoreType = True

    def isIgnored(self):
        return True
</code></pre>

<p><strong>On matter of Duck Typing with a Tell-Not-Ask policy</strong>:</p>

<p>I am new to Python, and would like to be a pythonic coder (and a better coder in general). Hence, </p>

<p>How do I <em>Duck Type</em> the above? Would checking the attribute value(<code>igIgnoreType</code>)/function(<code>isIgnored</code>) be considered <em>Duck Typing</em> if the attribute/function is never touched beyond object construction? </p>

<p>I do have another implementation, where <code>tidy</code> is overloaded in <em>Ignore</em> type nodes. <strong>No more type checking</strong>, but the parent still has to remove target child, and rebind grandkids. Here, the Ignore types return their children, which would be <code>[]</code> for leaf nodes. But, there is still a check on if the return was <code>None</code>. I am sure this is certainly <em>Duck Typing</em>, but is checking for <code>None</code> and code-replication, bad code?</p>

<pre class="lang-py prettyprint-override"><code>class ASTNode(object):
    def tidy(self):
        for i, child in reversed(list(enumerate(self.children))):
            grandkids = child.tidy()
            if grandkids is not None:
                self.children[i:i+1] = grandkids

        return None

class ASTIgnore(ASTNode):
    def tidy(self):
        for i, child in reversed(list(enumerate(self.children))):
            grandkids = child.tidy()
            if grandkids is not None:
                self.children[i:i+1] = grandkids

        return self.children
</code></pre>

<p><strong><em>_edit0</em></strong></p>

<p>Based on <a href="http://stackoverflow.com/users/84128/eric-conner">Eric's</a> vote, a <code>isIgnored()</code> function check implementation would look like</p>

<pre class="lang-py prettyprint-override"><code>def tidy(self):
    """
    Clean up useless nodes (ASTIgnore), and rebalance the tree
    Cleanup is done bottom-top
      in reverse order, so that the deletion/insertion doesn't become a pain
    """
    if self.children:
        # Only work on parents (non-leaf nodes)
        for i, child in reversed(list(enumerate(self.children))):
            # recurse, so as to ensure the grandkids are clean
            child.tidy()

            if child.isIgnored():
                grandkids = child.children
                self.children[i: i + 1] = grandkids
</code></pre>

<p>I think using the return value from the <code>tidy</code> method is a good way to pass information between your nodes. You're going to call <code>tidy</code> on each of your children anyway, so getting a return value that tells you what to do with that child makes the whole code simpler.</p>

<p>You can avoid repeating yourself by using <code>super</code> to call the base class's implementation from the derived class, and just changing the return value:</p>

<pre><code>class ASTIgnore(ASTNode):
    def tidy(self):
        super().tidy() # called for the side-effects, return value is overridden
        return self.children
</code></pre>

<p>If you're using Python 2, where <code>super</code> is a little bit less magical than it is in Python 3, you'll need to use <code>super(ASTIgnore, self)</code> instead of <code>super()</code>.</p>

python typechecking duck-typing