Python Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed

Question

I see this message from sonarlint and trying to figure out how to reduce the Cognitive Complexity of this function. Any assistance is appreciated in advance.

import os
import json
import click
import hcl

cfn = [".json", ".template", ".yaml", ".yml"]
tf  = ["tf"]

def file_handler(dir):
    for root, dirs, files in os.walk(dir):
        for file in files:
            if file.endswith(tuple(cfn)):
                with open(os.path.join(root, file), 'r') as fin:
                    try:
                        file = fin.read()
                        if "AWSTemplateFormatVersion" in file:
                            data = json.dumps(file)
                            print(data)

                    except ValueError as e:
                        raise SystemExit(e)

            elif file.endswith(tuple(tf)):
                with open(os.path.join(root, file), 'r') as file:
                    try:
                        obj  = hcl.load(file)
                        data = json.dumps(obj)
                        print(data)
                    except ValueError as e:
                        raise SystemExit(e)
    return data

What metrix tool are you using to get the resulting numbers? — SteveJ, Dec 27 '17 at 17:36
Extract functions, create handler lookups for file types, unify error handling, remove duplication, etc, etc. — Peter Wood, Dec 27 '17 at 17:37

Peter Wood · Accepted Answer · 2017-12-29T22:19:44.213

Extract function to yield file paths:

def _file_paths(directory):
    for root, dirs, filenames in os.walk(directory):
        for filename in filenames:
            file_path = os.path.join(root, filename)
            if os.path.isfile(file_path):
                yield file_path

Unify exception handling:

from contextlib import contextmanager

@contextmanager
def _translate_error(from_error, to_error):
    try:
        yield
    except from_error as error:
        raise to_error(error)

Extract functions to handle file types:

def _handle_cfn(file_object):
    file_contents = file_object.read()
    if "AWSTemplateFormatVersion" in file_contents:
        data = json.dumps(file_contents)
        print(data)


def _handle_tf(file_object):
    obj  = hcl.load(file_oject)
    data = json.dumps(obj)
    print(data)

Create a null handler for when the file extension doesn't match:

def _null_handler(file_object):
    pass

Map file extensions to handlers:

_extension_handlers = {'.json': _handle_cfn,
            '.template': _handle_cfn,
            '.yaml': _handle_cfn,
            '.yml': _handle_cfn,
            '.tf': _handle_tf}

Putting it together:

import os
import json
import click
import hcl

def file_handler(dir):
    for file_path in _file_paths(dir):
        base, extension = os.path.splitext(file_path)
        handler = _extension_handlers.get(extension, _null_handler)
        with open(file_path) as file_object:
            with _translate_error(ValueError, SystemExit):
                handler(file_object)

You can go further by extracting out a function for handling each file:

def _handle_file(file_path):
    base, extension = os.path.splitext(file_path)
    handler = _extension_handlers.get(extension, _null_handler)
    with open(file_path) as file_object:
        with _translate_error(ValueError, SystemExit):
            handler(file_object)

Then your main function is:

def file_handler(dir):
    for file_path in _file_paths(dir):
        _handle_file(file_path)

I'd recommend the second-to-last `file_handler` (non-`map` based); `map` is for functional programming, and shouldn't be used for side-effects (needlessly creating useless temporary `list`s on Python 2, and not working at all on Python 3 without additional changes). — ShadowRanger, Dec 27 '17 at 19:45
@ShadowRanger agreed, will remove without explanation, as it's more than needed. — Peter Wood, Dec 27 '17 at 20:35
This worked. def _file_paths(directory): for root, dirs, filenames in os.walk(directory): for filename in filenames: path = os.path.join(root, filename) if os.path.isfile(path): yield path — kilomo, Dec 29 '17 at 20:40

score 0 · Answer 2 · answered Dec 27 '17 at 17:39

You can remove one level of indentation by using two generator expressions for filtering the files by extension:

def file_handler(dir):
    for root, dirs, files in os.walk(dir):
        cfn_files = (file for file in files if file.endswith(tuple(cfn)))
        tf_files = (file for file in files if file.endswith(tuple(tf)))
        for file in cfn_files:
            with open(os.path.join(root, file), 'r') as fin:
                try:
                    file = fin.read()
                    if "AWSTemplateFormatVersion" in file:
                        data = json.dumps(file)
                        print(data)
                except ValueError as e:
                    raise SystemExit(e)
        for file in tf_files:
            with open(os.path.join(root, file), 'r') as file:
                try:
                    obj  = hcl.load(file)
                    data = json.dumps(obj)
                    print(data)
                except ValueError as e:
                    raise SystemExit(e)
    return data

nitred · Answer 3 · 2017-12-27T18:03:05.107

You should consider using glob for recursive file finding especially when you know what file extensions you're looking for:

import glob
import json
import os

import click
import hcl

def file_handler(dir):
    for extension in cfn:
        # eg: /path/to/dir/**/*.json
        glob_search = os.path.join(dir, "**/*{}".format(extension))  
        filenames = glob.glob(glob_search, recursive=True)

        for filename in filenames:
            with open(filename, 'r') as fin:
                try:
                    file = fin.read()
                    if "AWSTemplateFormatVersion" in file:
                        data = json.dumps(file)
                        print(data)

                except ValueError as e:
                    raise SystemExit(e)

    for extension in tf:
        # eg: /path/to/dir/**/*tf
        glob_search = os.path.join(dir, "**/*{}".format(extension))
        filenames = glob.glob(glob_search, recursive=True)

        for filename in filenames:
            with open(filename, 'r') as file:
                try:
                    obj = hcl.load(file)
                    data = json.dumps(obj)
                    print(data)
                except ValueError as e:
                    raise SystemExit(e)

FYI: A question about using glob (Use a Glob() to find files recursively in Python?)

Python Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed

3 Answers3