Roman Suzi
1 min readJan 11, 2020

--

A couple of comments. First, it’s not good idea to open a file directly from input without confining possible paths to specific directory or directories.

Second, there is a lot of unnecessary boilerplate code here:

        response_dict[STATUS] = "true
response_dict["labels_mapping"] = labels_dict
js_dump = json.dumps(response_dict)
resp = Response(js_dump,status=200,
mimetype='application/json')

You can use jsonify for the same purpose:

resp = jsonify(labels_mapping=labels_dict, STATUS=True)

Or you can easily write your own jsonify-like function to account for response status, mimetype, etc. Also there is nothing wrong to return the response right away instead of at the end of the function: In many cases it can help you keep indentation to the minimum and simplify understanding of the logic. Also in good code it is advisable to do actions elsewhere, not inside the view function as it violates SOLID single responsibility principle. In a view function makes sense only to check and convert input, check permissions, convert the result to output. This helps testability a lot. Unfortunately, Flask’s magical url_for, current_user, request, etc do not promote this style, but do not make it difficult as well.

There are also some typos. Eg,

response_dict[STATUS] = "true

you probably had

response_dict["STATUS"] = "true"

in mind.

And by the way StackExchange has a special are for code review. If you have working code, you can post a question at Code Review and receive useful comments and answers.

--

--

No responses yet