Should internal class methods return values or just modify instance variables?

instance-variables, oop, python

Solution

Returning a value is preferable as it allows you to keep all the attribute modifying in one place (`__init__`). Also, this makes it easier to extend the code later; suppose you want to override `_build_query` in a subclass, then the overriding method can just return a value, without needing to know which attribute to set. Here's an example:

class QueryHelper(object):
    def __init__(self, param, text):
        self._param = param
        self._query = self._build_query(text)

    def _build_query(self, text):
        return text + " and ham!"

class RefinedQueryHelper(QueryHelper):
    def _build_query(self, text):
        # no need to know how the query object is going to be used
        q = super(RefinedQueryHelper, self)._build_query()
        return q.replace("ham", "spam")

vs. the "setter version":

class QueryHelper(object):
    def __init__(self, param, text):
        self._param = param
        self._build_query(text)

    def _build_query(self, text):
        self._query = text + " and ham!"

class RefinedQueryHelper(QueryHelper):
    def _build_query(self, text):
        # what if we want to store the query in __query instead?
        # then we need to modify two classes...
        super(RefinedQueryHelper, self)._build_query()
        self._query = self._query.replace("ham", "spam")

If you do choose to set an attribute, you might want to call the method `_set_query` for clarity.

Problem

I am creating a query builder class that will help in constructing a query for mongodb from URL params. I have never done much object oriented programming, or designed classes for consumption by people other than myself, besides using basic language constructs and using django's built in Models. So I have this `QueryBuilder` class ``` class QueryHelper(): """ Help abstract out the problem of querying over vastly different dataschemas. """ def __init__(self, collection_name, field_name, params_dict): self.query_dict = {} self.params_dict = params_dict db = connection.get_db() self.collection = db[collection_name] def _build_query(self): # check params dict and build a mongo query pass ``` Now in `_build_query` I will be checking the `params_dict` and populating `query_dict` so as to pass it to mongo's `find()` function. In doing this I was just wondering if there was an absolute correct approach to as whether `_build_query` should return a dictionary or whether it should just modify `self.query_dict`. Since it is an internal method I would assume it is OK to just modify `self.query_dict`. Is there a right way (pythonic) way of approaching this? Is this just silly and not an important design decision? Any help is appreciated.

Original source