Should I create a class if my function is complex and has a lot of variables?

Question

This question is somewhat language-agnostic, but not completely, since Object Oriented Programming (OOP) is different in, for example, Java, which doesn't have first-class functions, than it is in Python.

In other words, I feel less guilty for creating unnecessary classes in a language like Java, but I feel like there might be a better way in the less boilerplate-y languages like Python.

My program needs to do a relatively complex operation a number of times. That operation requires a lot of "bookkeeping", has to create and delete some temporary files, etc.

That's why it also needs to call a lot of other "suboperations" - putting everything into one huge method isn't very nice, modular, readable, etc.

Now these are approaches that come to my mind:

1. Make a class that has one public method only and keeps the internal state needed for the suboperations in its instance variables.

It would look something like this:

class Thing:

    def __init__(self, var1, var2):
        self.var1 = var1
        self.var2 = var2
        self.var3 = []

    def the_public_method(self, param1, param2):
        self.var4 = param1
        self.var5 = param2
        self.var6 = param1 + param2 * self.var1
        self.__suboperation1()
        self.__suboperation2()
        self.__suboperation3()


    def __suboperation1(self):
        # Do something with self.var1, self.var2, self.var6
        # Do something with the result and self.var3
        # self.var7 = something
        # ...
        self.__suboperation4()
        self.__suboperation5()
        # ...

    def suboperation2(self):
        # Uses self.var1 and self.var3

#    ...
#    etc.

The problem I see with this approach is that the state of this class makes sense only internally, and it can't do anything with its instances except call their only public method.

# Make a thing object
thing = Thing(1,2)

# Call the only method you can call
thing.the_public_method(3,4)

# You don't need thing anymore

2. Make a bunch of functions without a class and pass the various internally needed variables between them (as arguments).

The problem I see with this is that I have to pass a lot of variables between functions. Also, the functions would be closely related to each other, but they wouldn't be grouped together.

3. Like 2. but make the state variables global instead of passing them.

This would be no good at all, since I have to do the operation more than once, with different input.

Is there a fourth, better, approach? If not, which one of these approaches would be better, and why? Is there something I'm missing?

Answer 1

1. Make a class that has one public method only and keeps the internal state needed for the suboperations in its instance variables.

The problem I see with this approach is that the state of this class makes sense only internally, and can't do anything with its instances except call their only public method.

Option 1 is a good example of encapsulation used correctly. You want the internal state to be hidden from outside code.

If that means your class only has one public method, then so be it. It'll be that much easier to maintain.

In OOP if you have a class that does exactly 1 thing, has a small public surface, and keeps all its internal state hidden, then you are (as Charlie Sheen would say) WINNING.

2. Make a bunch of functions without a class and pass the various internally needed variables between them (as arguments).

The problem I see with this is that I have to pass a lot of variables between functions. Also, the functions would be closely related to each other, but wouldn't be grouped together.

Option 2 suffers from low cohesion. This will make maintenance more difficult.

3. Like 2. but make the state variables global instead of passing them.

Option 3, like option 2, suffers from low cohesion, but much more severely!

History has shown that the convenience of global variables is outweighed by the brutal maintenance cost it brings. That's why you hear old farts like me ranting about encapsulation all the time.

---

The winning option is #1.

Answer 2

I think #1 is actually a bad option.

Let's consider your function:

def the_public_method(self, param1, param2):
    self.var4 = param1
    self.var5 = param2 
    self.var6 = param1 + param2 * self.var1
    self.__suboperation1()
    self.__suboperation2()
    self.__suboperation3()

Which pieces of data does suboperation1 use? Does it collect data used by suboperation2? When you pass data around by storing it on self, I cannot tell how the pieces of functionality are related. When I look at self, some of the attributes are from the constructor, some from the call to the_public_method, and some randomly added in other places. In my opinion, its a mess.

What about number #2? Well, firstly, let's look at the second problem with it:

Also, the functions would be closely related to each other, but wouldn't be grouped together.

They would be in a module together, so they would totally be grouped together.

The problem I see with this is that I have to pass a lot of variables between functions.

In my opinion, this is good. This makes explicit the data dependencies in your algorithm. By storing them whether in global variables or on a self, you hide the dependencies and make them seem less bad, but they are still there.

Usually, when this situation arises, it means that you've not found the right way to decompose your problem. You are finding it awkward to split into multiple functions because you are trying to split it the wrong way.

Of course, without seeing your actual function its hard to guess at what would be a good suggestion. But you give a bit of hint of what you are dealing with here:

My program needs to do a relatively complex operation a number of times. That operation requires a lot of "bookkeeping", has to create and delete some temporary files etc.

Let me pick an example of something that sorta fits your description, an installer. An installer has to copy a bunch of files, but if you cancel it part way through, you need to rewind the whole process including putting back any files that you replaced. The algorithm for this looks something like:

def install_program():
    copied_files = []
    try:
        for filename in FILES_TO_COPY:
           temporary_file = create_temporary_file()
           copy(target_filename(filename), temporary_file)
           copied_files = [target_filename(filename), temporary_file)
           copy(source_filename(filename), target_filename(filename))
     except CancelledException:
        for source_file, temp_file in copied_files:
            copy(temp_file, source_file)
     else:
        for source_file, temp_file in copied_files:
            delete(temp_file)

Now multiply that logic out for having to do registry settings, program icons, etc, and you've got quite a mess.

I think that your #1 solution looks like:

class Installer:
    def install(self):
        try:
            self.copy_new_files()
        except CancellationError:
            self.restore_original_files()
        else:
            self.remove_temp_files()

This does make the overall algorithm clearer, but it hides the way the different pieces communicate.

Approach #2 looks something like:

def install_program():
    try:
       temp_files = copy_new_files()
    except CancellationError as error:
       restore_old_files(error.files_that_were_copied)
    else:
       remove_temp_files(temp_files)

Now its explicit how pieces of the data move between functions, but it is very awkward.

So how should this function be written?

def install_program():
    with FileTransactionLog() as file_transaction_log:
         copy_new_files(file_transaction_log)

The FileTransactionLog object is a context manager. When copy_new_files copies a file it does it through the FileTransactionLog which handles making the temporary copy and keep track of which files have been copied. In the case of exception it copies the original files back, and in the case of success it deletes the temporary copies.

This works because we've found a more natural decomposition of the task. Previously, we were intermixing the logic about how to install the application with the logic about how to recover a cancelled install. Now the transaction log handles all of the details about temporary files and bookkeeping, and the function can focus on the basic algorithm.

I suspect that your case is in the same boat. You need to extract out the bookkeeping elements into some sort of object so that your complex task can expressed more simply and elegantly.

Answer 3

Since the only apparent drawback of method 1 is its suboptimal usage pattern, I think the best solution is to drive encapsulation one step further: Use a class, but also provide a free standing function, that only constructs the object, calls the method and returns:

def publicFunction(var1, var2, param1, param2)
    thing = Thing(var1, var2)
    thing.theMethod(param1, param2)

With that, you have the smallest possible interface to your code, and the class that you use internally really becomes just an implementation detail of your public function. Calling code never needs to know about your internal class.

Answer 4

On the one hand, the question is somehow language agnostic; but on the other hand, the implementation depends on the language and its paradigms. In this case it is Python, which supports multiple paradigms.

Besides your solutions, there is also a possibility, to do the operations complete stateless in a more functional way, e.g.

def outer(param1, param2):
    def inner1(param1, param2, param3):
        pass
    def inner2(param1, param2):
        pass
    return inner2(inner1(param1),param2,param3)

It all comes down to

• readability
• consistency
• maintainability

But -If your codebase is OOP, it violates consistency if suddenly some parts are written in a (more) functional style.

Answer 5

Why Design when I can be Coding?

I offer a contrarian view to the answers I've read. From that perspective all the answers and frankly even the question itself focus on coding mechanics. But this is a design issue.

Should I create a class if my function is complex and has a lot of variables?

Yes, as it makes sense for your design. It may be a class unto itself or part of some other class or it's behavior may be distributed among classes.

---

Object Oriented Design is about Complexity

The point of OO is to successfully build and maintain large, complex systems by encapsulating the code in terms of the system itself. "Proper" design says everything is in some class.

OO design naturally manages complexity primarily via focused classes that adhere to the single responsibility principle. These class give structure and functionality throughout the breath and depth of the system, interact and integrate these dimensions.

Given that, it is often said that functions dangling about the system - the all too ubiquitous general utility class - is a code smell suggesting an insufficient design. I tend to agree.

Answer 6

Why not compare your needs to something that exists in the standard Python library, and then see how that is implemented?

Note, if you don't need an object, you can still define functions within functions. With Python 3 there is the new nonlocal declaration to allow you to change variables in your parent function.

You might still find it useful to have some simple private classes inside your function to implement abstractions and tidying up operations.