Shouldn't we expand calls to class methods when evaluating cohesion?

[portusato]

Suppose you have this module:

class Data(object):
    def __init__(self, data):
        if not isinstance(data, list):
            raise ValueError('Data: Input arg must be list')
        self.data = data

    def get_value_at_start(self):
        return self.get_value_at_index(0)

    def get_value_at_index(self, index):
        return self.data[index]

data = Data([1, 3, 5, 7])
print 'Data 0: {}'.format(data.get_value_at_start())
print 'Data 2: {}'.format(data.get_value_at_index(2))

cohesion gives 0.00% for Function: get_value_at_start. IMO this is incorrect. By the definition you have in your documentation (extracted from wikipedia): "[...] cohesion refers to the degree to which the elements of a module belong together". Getting a cohesion of 0% would mean that the method does not belong to the class. However, it does; it's only that we're avoiding code repetition. If the function did 'return self.data[0]' we'd get cohesion score=100%, but this would go against good practices, because if later we change something in how we access data points, we need to change it in two places.
What do you think?

[mschwager]

Hmm, this is an interesting case.

cohesion doesn't currently have the smarts to include sub-method call cohesion along with the cohesion of a specific method. Meaning, exactly what you said: it only includes bounded variables within a class method, not the bounded variables within sub-method calls of a class method.

Initially, I thought changing to the suggested method for determining cohesion would be a big lift. It seems like a tree-like structure would work best for this type of analysis. A class method's sub-method calls could simply be child nodes in a tree, which would make aggregating cohesion information simple. cohesion currently uses lists internally to track this information. However, getting back to my initial point, I now do not think it'd be too big of a lift to change to the new, suggested method of determining a class's cohesion.

I would like to think about this new approach a little bit more, though.

If the function did 'return self.data[0]' we'd get cohesion score=100%, but this would go against good practices, because if later we change something in how we access data points, we need to change it in two places.

I think this is true if we're only considering DRYness when thinking about code quality. However, it doesn't seem unreasonable to me that internal data within a class is used directly vs. indirectly accessing via other class methods. If we think about cohesion more abstractly, I think it can be useful to understand a particular method's cohesion, and thus sense of belonging, within a class without considering the cohesion of its sub-methods. I'm not sure that there's a right and wrong answer here, maybe just personal preference? Perhaps a command-line flag is the best solution :)

Thoughts?