# Make django, weasyprint code object oriented

Please ignore the comments in the code for the purpose of reviewing. I just require tips on using SOLID principles and how classes can be used in this code.Comments in the code are for mapping the image in my brain to what I want to do in words. Sorry for the mistakes in the comments of the code. I wrote them really fast.

I followed the instructions and refactored my code as per the suggestions by @Reinderien but recently the client said to me that on the webpage there are n ID's and n different PDF's are to be returned based on which ID the user selects. Currently n=2. I have a radio select of which the visitor can select any one of the ID's and enter the correct value of input. So basically I get the value of the radio button as opt and value as the actual ID value. Now I have to check the database to find the column which is named as opt and the row where value in that column is value. Initially there was just one form/word_document/.html_file that was being converted to PDF but now there are three forms. I know it's easy at the moment but I think I need some ideas on whether use of class should be done and if so then how? I 'm really interested in refactoring this code and use SOLID principles. Surely, I 'm beginner level with design patterns and SOLID principles but it's just more and more code reviews that will make my eye catch 'how much' 'single responsibility' a function should be and if really if my function is extensible w/o modifying it. Please assume I 'm good enough at decorator,inheritance and class composition. Also I couldn't get a way to remove the making of temporary files. There is temporary saving of the PDF made by weasyPrint. I don't want that you code it up for me. Just few lines of explanation or a small UML diagram would be more than sufficient.

vars.cfg

[FILES]
HTML_FILE_NAME_FOL=C:\Users\Dell\Desktop\gjh\formfill\media\mydoc-utf8.htm
HTML_FILE_NAME_DPID=C:\Users\Dell\Desktop\gjh\formfill\media\mydoc-utf8.htm
EXCEL_FILE_NAME=C:\Users\Dell\Desktop\gjh\formfill\media\AM2.CSV


forms.py

from django import forms

class InputData(forms.Form):
opt = forms.ChoiceField(label="Enter Data:", choices=[('FOL', "Folio Number"),
('DPID', 'Dpid')
],

value = forms.CharField(label="Value ", widget=forms.TextInput(
attrs={'pattern': '[A-Z0-9]{0,16}'}))


backend.py(improved)

import os

import pandas as pd
import codecs
from weasyprint import HTML

import configparser
import tempfile

from django import template
from pathlib import Path

def remove_temp_file():
if os.path.exists('temp.pdf'):
os.remove('temp.pdf')

def get_config_object():
config = configparser.RawConfigParser()
config.optionxform = str
return config

config = get_config_object()

""" TODO : Django by its nature will only convert a 'template' to HTML.
For that the file is being saved to the default location that is the template folder.
I just observed that I do now need to run this function again and again but once once similar
to like what one would do if one wants to fill the database the first time.
Is this possible anyway? I have to give the project folder to the client. He will place all the forms
paths in the cfg file.

vars.cfg
[FILES]
HTML_FILE_NAME_FOL=C:\Users\Dell\Desktop\gjh\formfill\media\mydoc-utf8.htm
HTML_FILE_NAME_DPID=C:\Users\Dell\Desktop\gjh\formfill\media\mydoc-utf8.htm
EXCEL_FILE_NAME=C:\Users\Dell\Desktop\gjh\formfill\media\AM2.CSV

"""

html = codecs.open(
config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"],
html += "{% load numbersinwords %}"

with open(config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"], "w", encoding="utf-8") as html_file:
html_file.write(html)

def html2pdf(row, path_form):
row = row.to_dict()
html = render_to_string(Path(path_form).name,
{key: row[value]
for key, value in config._sections["TAGS"].items()})
return html

def get_data():
dtype=str, keep_default_na=False)

def search_row(opt, value):
user_data = get_data()
return user_data[user_data[opt] == value]

def main(opt, value):

remove_temp_file()

row = search_row(opt, value)

if len(row) == 1:
row = row.squeeze()
else:
return (False, f"<h1>Invalid credential :"
" Multiple candidates exists"
"with given credential</h1>")

if not(row.empty):
html = html2pdf(row, Path(config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"]))
HTML(string=html).write_pdf("temp.pdf")

f = open("temp.pdf", "rb")

return (True, f)

return (False, f"<h1>Invalid credential {opt}: {value}</h1>")


## Configuration file paths

If possible, factor out a common path:

[FILES]
MEDIA_PATH=C:\Users\Dell\Desktop\gjh\formfill\media
HTML_FILE_NAME_FOL=mydoc-utf8.htm
HTML_FILE_NAME_DPID=mydoc-utf8.htm
EXCEL_FILE_NAME=AM2.CSV


## Typo?

I do now need -> I do not need

## Caching

I do [not] need to run this function again and again but once once similar to like what one would do if one wants to fill the database the first time. Is this possible anyway?

Almost certainly. The easiest way to do this is check if the file exists beforehand. One risk of this approach is that it may not be thread-safe, so you may need to surround the file-check-file-write in a lock if access to that code path is multi-threaded.

## Enums

Consider making an Enum to represent the two (?) choices for opt values:

class TagOpt(Enum):
FOL = 'FOL'
DPID = 'DPID'


rather than accepting it as a string.

## Temporary variables

config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"]


should be put into a temporary variable since you write it twice; perhaps:

html_path = config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"]
with open(html_path, "w", encoding="utf-8") ...


## I/O costs

Depending on the load characteristics of your application, you might want to modify load_custom_tags to use something like an LRU cache so that a certain number of most-recently-used HTML files are kept in memory. The cost of a round trip to the hard disc might end up being inconvenient to pay.

• Sorry I should have explained it better but soon will with experience on how to present question better. I added those comments so that the reviewer can understand my code better. I ll just add clarification to ignore the comments. Sorry for the trouble @Reinderien – Vishesh Mangla Jun 29 '20 at 19:21
• 1) ENUM :where @Reinderien. In the forms.py or the backend.py? 2)Temporary variables: like temp =config["FILES]; temp[".."]? – Vishesh Mangla Jun 29 '20 at 19:31
• Wow, that was some amazing info there, I didn't know that LRU cache works for file objects too. – Vishesh Mangla Jun 29 '20 at 19:32
• LRU cache works for file objects too - careful. I would not apply an LRU cache to a file handle, but that isn't what's going on here. Instead, you would apply an LRU cache to a string or enum that uniquely identifies the file. – Reinderien Jun 29 '20 at 19:34
• In the forms.py or the backend.py? - backend.py – Reinderien Jun 29 '20 at 19:35

No need of f strings at places required where no placeholders are there

return (False, f"<h1>Invalid credential :"
" Multiple candidates exists"
"with given credential</h1>")


should be simply

return (False, "<h1>Invalid credential :"
" Multiple candidates exists"
"with given credential</h1>")


Open close Principle violated:

view.py is dependent on what type of response is being sent back from backend.py. This is bad since if more type of responses are to be returned like HttpResponse, FileResponse, JsonResponse the technique of True False can't be used. You need more than 0/1 in this case. This is not extensible code. The view cares about the response being sent.

This should be better:

view.py

def index(request):
if request.method == "POST":
form = InputData(request.POST)
if form.is_valid():
return backend.main(**form.cleaned_data)

form = InputData()

return render(request, "base/index.html", {
'forms': form
})



backend.py

import os

import pandas as pd
from weasyprint import HTML

import configparser

from django.http import HttpResponse, FileResponse
from django.conf import settings

def get_configurations():
config = configparser.RawConfigParser()
config.optionxform = str
return config

config = get_configurations()

def remove_temp_file():
if os.path.exists('temp.pdf'):
os.remove('temp.pdf')

def get_html(opt, row):

row = row.to_dict()
file_path = os.path.join(settings.MEDIA_ROOT,
config["FILES"][f"HTML_FILE_NAME_{opt.upper()}"])
return render_to_string(file_path, context=row)

def search_row_in_database(opt, value):
config["FILES"]["EXCEL_FILE_NAME"]),
dtype=str, keep_default_na=False)
return df[df[opt] == value]

def get_pdf(opt, row):
html = get_html(opt, row)
HTML(string=html).write_pdf("temp.pdf")
f = open("temp.pdf", "rb")
return f

def main(opt, value):

remove_temp_file()

row = search_row_in_database(opt, value)

# check if a single row with that ID exists
if len(row) == 1:
row = row.squeeze()
return FileResponse(
# opt is required to choose which html pdf is to be picked up
get_pdf(opt, row),
as_attachment=True,
# no rows with that ID found
elif len(row) == 0:
return HttpResponse("<h1>Invalid credential {opt}: {value}. "
"No user with that ID found</h1>")
# in case of not multiple rows with that ID
else:
return HttpResponse("<h1>Invalid credential :"
" Multiple candidates exists"
"with given credential</h1>")


Since the html file is only created once the tag {% load numbersinwords %} should not be put like this.It should be put on creation of the html file manually. These tags can be stored in a readme.md often used with git. The user of the code should then read the readme and put the tag at the top of his html form rather than you doing it programmatically.

• I expect such type of answers to this question. Obviously I' m not referring to the f strings . – Vishesh Mangla Jul 2 '20 at 21:53