Know too much

Single responsibility principle is stating that given program, procedure or function should be responsible for one thing. Following this rule means also that we should think about how much our part of the program know, and how much is enough to perform given operation.

Optional arguments

Optional arguments are values which can be passed to given function or not, optional argument can also mean the value can be None or null or any other empty value (depends from the language). Let's see first code example, it will by Python with some MyPy type annotations, but don't worry if you don't know Python, examples should be easy for coder of any language as I have tried to keep complexity in very low level.

def buy_product(user: User = None, product: Product = None):
  if user and product:
    user.products.add(product)

Function buy_product has two optional arguments, and it means it can be called without anything, or with None values. Below some examples of such "strange" calling of the function:

buy_product() # yyy, but what product and who buys it?
buy_product(user=None, product=None) # buying None by None :)
buy_product(user=my_user, product=None) # ah yes user buys nothing
buy_product(user=None, product=my_product) # ah yes None buys smth

I hope when you read above code you feel uncomfortable, at least I do. Something is wrong here, how we can buy product without providing who buys it neither what is being bought.

The problem with buy_product is that it has really no answer on situation where one of arguments is not set, this function for None in any of arguments changes in noop (No Operation) function, so does nothing.

The intention of a maker of such function is to call it in any circumstances, and be sure that it will work only for valid data, and be noop in other situations. This also avoids branching logic, we always call it. But do we really avoid anything? And what if we will pass optional value down even more?

def do_payment(user: User = None, product: Product = None):
  if user and product:
    call_some_payment_service(...)

def add_product_db(user: User = None, product: Product = None):
  if user and product:
    append_to_db(...)

def log(user: User = None, product: Product = None):
  if user and product:
    log_service_call(...)

def buy_product(user: User = None, product: Product = None):
  do_payment(user, product)
  add_product_db(user, product)
  log(user, product)

Now our situation changes even more. We still can just call our function buy_product() but it means that it passes None down and all functions deal with it.

The problem with all of this is that all functions can work only with existing values, are not prepared to work with None, like None is not supported value of these functions domains. These functions know too much, they know that argument which is required for them to work can be not set. By doing that we extend all functions responsibilities to handle optionality. We don't want that, believe me.

The solution for this problem is - call these functions only when we have enough data to perform the operation.

# definition without optionality in arguments
def buy_product(user: User, product: Product):
  # below functions also do not consider working with None
  do_payment(user, product)
  add_product_db(user, product)
  log(user, product)


# calling it
if user and product:
  buy_product(user, product)

In result all data passed below buy_product is safe as it is always set. No need to handle optionality anymore.

Send some email

Our codebase has such a function for email sending.

def send_email(user, email_template, data):
  if user.active and user.email_notifications:
    smpt_conf = {# setting some email conf #}
    content = compile_template(email_template, data)
    my_email_service(
      smpt_conf, 
      user.email, 
      content, 
      title = data['title']
    )

Now new requirements comes and we need to send email to not a user, but somebody else. We have email, title, and even content which is compiled html. But... we clearly cannot use our send_email function as it is prepared only for users and only for some specific templates.

And this is because function which should just send email knows about users, knows when user can get email, knows how to generate email template. It knows too much.

def send_email(email, title, content):
  smpt_conf = {# setting some email conf #}
  my_email_service(smpt_conf, email, content, title)

The new version is simplified, has only knowledge about email things. Knows what and to what email should be send. Now it can be used in the new use case:

send_email(my_email, my_title, my_html)

And now using with user

def send_email_to_user(user, email_template, data):
  if user.active and user.email_notifications:
    send_email(
      user.email, 
      data['title'], 
      compile_template(email_template, data)
    )

Done, we have made send_email responsible only for sending emails, and send_email_to_user is domain related function which deals with sending emails to users.

And now small addition, if we take a look at send_email_to_user also is noop when our user is not active or has no email notifications turn on, why then this function needs to know it? Well, in this particular situation it can be fine to have function which is responsible for sending user emails, and sending it only when it is possible, we have this additional safety that email will not be send if should not be. Having function which sometimes does nothing is a tool which we should use but not overuse.

Am I logged in?

log_movement(request)
add_points(request)
send_sms(request)

Definitions of above functions:

def log_movement(request: Request):
  if request.session:
    log("movement", request.session.user.name)
  else:
    log("noop",'Anonymous user')

def add_points(request: Request):
  if request.session:
    db_add_points(request.session.user)

def send_sms(request: Request):
  if request.session and request.session.user.sms_notifications:
    sms_service("movement", request.session.user)

Now, what is wrong? Take a look that all these functions have the same problem and the same condition, they check if user has session and perform operation. Request object contains a lot of other informations, which are really not important for these functions, and if we would like to test them, we would need to mock all these details, mocking not used things is not specially joyful experience . Again, these functions know too much, let's fix it.

# definitions with less knowledge
def log_movement(user_name: string):
  log("movement", user_name)

# below function even not need to exists 
# as it just calls another
def add_points(user: User):
  db_add_points(user)

def send_sms(user: User):
  if user.sms_notifications:
    sms_service("movement", user)

# calling them
if request.session:
  user = request.session.user
  log_movement(user)
  add_points(user)
  send_sms(user)
else:
  log("noop",'Anonymous user')

Instead of moving responsibility to all functions, we just deal with the question once on the top, other functions have only what they wanted, nothing more.

In summary, if we can narrow functions domain and knowledge, we should do it, instead of passing the problem down, deal with it as soon as it can be solved. If function does not work for None do not use it with it, if many functions deal with the same problem, maybe problem can be solved before they are called? Of course it depends, and every choice you make in the code should be carefully considered, as there are no silver bullets.