When does a function violate the SRP?

Question

I have a function that was written because several methods used the same sequence of code so an abstraction was useful. The function takes a string representing the path to a registry key and then a list of 3-tuple containing attributes to update the key.

This function is subsequently called updateKeyAttributes:

_ROOT_MASK = {HKEY_CLASSES_ROOT: 'HKEY_CLASSES_ROOT',
              HKEY_CURRENT_USER: 'HKEY_CURRENT_USER'}

def updateKeyAttributes(hiveroot: HKType, key: str, attrs: Sequence[Tuple[Optional[str], int, Optional[Any]]) -> None:

    try:
        with CreateKeyEx(hiveroot, key, 0, KEY_WRITE) as open_key:
            for name, typ, value in attrs:
                SetValueEx(open_key, name, 0, typ, val)
    except OSError as err:
        path = f'{_ROOT_MASK[hiveroot]\\{key}'
        raise RegistryError(f'Failed to update key attributes: {path}') from err

As I was looking at this function, I started to wonder if it is violating the SRP?

To me, its responsibility is to "update the attributes of a particular key", and that it's only job. However, how granular does that "single responsibility" go?

The body of the function does 3 things:

1. Creates (subsequently Opens) a handle to an HKey
2. Sets the values of the attributes for the HKey
3. Raises a wrapped exception if #1 or #2 fail

Is the SRP that granular or is my understanding that the function is only responsible for updating, and conversely not deleting, valid within the SRP? I've read that SRP only applies to how classes behave together in a program, but not about whether it relates to how standalone methods. Surely at some point a method does something more than just a single thing:

def main():

    program_init()
    program_run()
    program_cleanup()

Unless your granularity is "it managed the programs lifespan"...

Edit:

In the event it is a violation, I can pull some out:

def updateKeyAttributes(key: HKType, attrs: Sequence[Tuple[Optional[str], int, Optional[Any]]) -> None:
    
    for name, typ, val in attrs:
        SetValueEx(key, name, 0, typ, val)

try:
    with CreateKeyEx(hiveroot, key, 0, KEY_WRITE) as open_key:
        updateKeyAttributes(open_key, attrs)
except OSError as err:
    path = f'{_ROOT_MASK[hiveroot]\\{key}'
    raise RegistryError(f'Failed to update key attributes: {path}') from err

But now the module will have, two try/except blocks rather than sharing a common function.

Answer 1

SRP is a Rule of Thumb. Its amazing how many thumbs it does not fit.

Take it as a guide, not as a law of nature.

---

I'd argue that the most important principal is Readability. SRP being one of its horsemen, but not the main show.

I argue that Readability is more important because the code is read by many people and many machines, many times over.

SRP can help get us to more readable code, but like you've highlighted it can go circular and lead us back out to worse code.

Think of normal forms in a database. Each normal form from 1st through to 3rd slowly improves the quality of the data structure and generally makes it more legible. But as you keep normalising you will arrive at the Triple store. A Triple can store any data, it is arguably the most normalised, and universal schema you can have. When you don't know what you are storing, its the best option.

It is however also the most terrible data-structure to have. Any question you have about the organisation or structure of the data has to be discovered by trawling through all the data in the store, or at least the relevant parts. You have no upfront certainty and must double check every assumption.

Similarly in code you want to "normalise"/apply SRP while it is improving the code quality judged by how easy it is to read and understand. You know its easier to understand because the structure tells you the answer without having to execute the program.

The moment SRP starts reducing that code quality, ignore it. You'll know this because you will have to run the program to discover how it hangs together. Not that you can remove this need altogether, it is a program after all, not a lookup table. But given the choice between having to try to understand by looking at the result, and understanding by reading the code itself, prefer reading the code.

Answer 2

SRP granularity should match the granularity of your logic. You deal with registry items, then that is the abstraction level for your program. Anything lower level is covered for you by OS/library functions so it is pointless to replicate that in your program. At the other side you do not want to mix dealing with the registry with your own methods so you isolate the registry item handling in a way that allows you to store and address the things that matter to you.

If your logic only stores and retrieves strings you may want to eliminate the item type from the argument list because it is meaningless to the logic that calls the method. You do want to see a name and a value in an argument list, tuples are not the best fit here, you don't want to create puzzles.

Answer 3

Look through recent posts and you will find one about a team lead with fanatical dedication to SRP who destroyed a company’s code base.

Never apply SRP because you are told to apply SRP as the only reason. It’s the most destructive rule ever invented. It has caused untold damages. You are looking for artificial reasons to apply SRP. In reality, but the truth is, your function has one responsibility only.

Think about it: You wrote this code. Are you going to ask someone else to press the “save” button because that is a separate responsibility? And a third person to try it out? No, it’s one responsibility to add a tested function to your source code control,organising all the needed reviews.