6
\$\begingroup\$

I am learning to work with OOP design patterns and so I challenged myself with the idea of creating a way to generate a pdf invoice based on some information entered. So, this is what I have done so far and would like to have some review on my approach and design.

Invoice.py

import fpdf
from datetime import datetime
class Creator:
 def __init__(self,first_name,last_name,email,phone_num,address,city,country):
 self.first_name = first_name
 self.last_name = last_name
 self.email = email
 self.phone_num = phone_num
 self.address = address
 self.city = city
 self.country = country
class Organization:
 def __init__(self,name,address,city,country):
 self.name = name
 self.address = address
 self.city = city
 self.country = country
class BankAccountDetail:
 def __init__(self,account_name,account_num,currency,bank_name,branch,branch_addr):
 self.account_name = account_name
 self.account_num = account_num
 self.currency = currency
 self.bank_name =bank_name
 self.branch = branch
 self.branch_addr = branch_addr
class Project:
 def __init__(self,name,description,amount):
 self.name = name
 self.description = description
 self.amount = amount
class Invoice:
 '''
 Invoice class used to model a invoice object which is a composition of
 1. Creator Object
 2. Organization Object
 3. Project Object
 4. BankDetail Object
 '''
 def __init__(self,invoice_num,creator,organization,project,bankaccountdetail):
 self.invoice_num = invoice_num
 self.creator = creator
 self.organization = organization
 self.project = project
 self.bankaccountdetail = bankaccountdetail
class File:
 def __init__(self,filename,font_size,line_height,orientation):
 self.filename = filename
 self.font_size = font_size
 self.line_height = line_height
 self.orientation = orientation
class PdfInvoice(Invoice):
 '''
 Inherits from the Parent Invoice class and has an extra feature
 1. File Object : Used to specify some basic details about the file
 '''
 def __init__(self,invoice_num,creator,organization,project,bankaccountdetail,file):
 super().__init__(invoice_num,creator,organization,project,bankaccountdetail)
 self.file = file
 def generate_pdf(self):
 dt = datetime.now()
 date = dt.date()
 pdf = fpdf.FPDF(format=self.file.orientation)
 pdf.add_page()
 pdf.set_font("Arial", size=self.file.font_size)
 pdf.write(self.file.line_height,"Invoice Number #")
 pdf.write(self.file.line_height,self.invoice_num)
 pdf.ln()
 pdf.write(self.file.line_height,"Date Invoiced #")
 pdf.write(self.file.line_height,str(date))
 pdf.ln()
 pdf.write(self.file.line_height, "Billed By #")
 pdf.write(self.file.line_height,"{}{}".format(self.creator.first_name,self.creator.last_name))
 pdf.ln()
 pdf.write(self.file.line_height,"Address #")
 pdf.write(self.file.line_height,self.creator.address)
 pdf.ln()
 pdf.write(self.file.line_height, "City #")
 pdf.write(self.file.line_height, self.creator.city)
 pdf.ln()
 pdf.write(self.file.line_height,"Country #")
 pdf.write(self.file.line_height,self.creator.country)
 pdf.ln()
 pdf.write(self.file.line_height, "Email #")
 pdf.write(self.file.line_height, self.creator.email)
 pdf.ln()
 pdf.write(self.file.line_height, "Phone Number #")
 pdf.write(self.file.line_height, self.creator.phone_num)
 pdf.ln()
 pdf.write(self.file.line_height,"Billed To #")
 pdf.ln()
 pdf.write(self.file.line_height,"Organization Name #")
 pdf.write(self.file.line_height,self.organization.name)
 pdf.ln()
 pdf.write(self.file.line_height, "Organization Address #")
 pdf.write(self.file.line_height, self.organization.address)
 pdf.ln()
 pdf.write(self.file.line_height, "Organization City #")
 pdf.write(self.file.line_height, self.organization.city)
 pdf.ln()
 pdf.write(self.file.line_height, "Organization Country #")
 pdf.write(self.file.line_height, self.organization.country)
 pdf.ln()
 pdf.write(self.file.line_height, "Comments #")
 pdf.write(self.file.line_height, self.project.description)
 pdf.ln()
 pdf.write(self.file.line_height, "Amount #")
 pdf.write(self.file.line_height,str(self.project.amount))
 pdf.ln()
 pdf.write(self.file.line_height,'Account details ')
 pdf.ln()
 pdf.write('Account Name #')
 pdf.write(self.file.line_height,self.bankaccountdetail.account_name)
 pdf.ln()
 pdf.write('Account Number #')
 pdf.write(self.file.line_height,self.bankaccountdetail.account_num)
 pdf.ln()
 pdf.write('Account Currency #')
 pdf.write(self.file.line_height, self.bankaccountdetail.currency)
 pdf.ln()
 pdf.write('Bank Name #')
 pdf.write(self.file.line_height, self.bankaccountdetail.bank_name)
 pdf.ln()
 pdf.write('Branch Address #')
 pdf.write(self.file.line_height, self.bankaccountdetail.branch_addr)
 pdf.ln()
 pdf.output(self.file.filename)
creator = Creator('Test','User','[email protected]',
 '099006789','Joans Apartment, 123 Test road','Nairobi','Kenya')
organization = Organization('Test Org','Ndemi Road Kilimani', 'Nairobi','Kenya')
bank_detail = BankAccountDetail('Test User','999999678','KES',
 'Test Bank','Kenya','BRANCH Way, ABC Place')
file = File("Invoice.pdf",12,5,"letter")
project = Project('Ecommerce site','Worked on the ecommerce site',10.900)
pdf_inv = PdfInvoice('1393939',creator,organization,project,bank_detail,file)
pdf_inv.generate_pdf()

UML diagram

Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Jun 14, 2019 at 13:34
\$\endgroup\$
4
  • \$\begingroup\$ Welcome to Code Review! Is this supposed to work in Python 2 or Python 3? Please add the according tag to your question. \$\endgroup\$ Commented Jun 14, 2019 at 14:03
  • \$\begingroup\$ @AlexV:- Thank you. I have added the tag. \$\endgroup\$ Commented Jun 14, 2019 at 14:18
  • 1
    \$\begingroup\$ Rendering text onto the PDF using one statement per string is quite tedious. Consider using a higher-level library or templating language such as ReportLab. \$\endgroup\$ Commented Jun 14, 2019 at 18:09
  • \$\begingroup\$ Welcome to Code Review! Please see What to do when someone answers. I have rolled back Rev 5 → 4 \$\endgroup\$ Commented Jun 14, 2019 at 22:21

2 Answers 2

7
\$\begingroup\$

Data classes

Since you are using the classes as immutable data containers, it would be possible to significantly cut down the amount of code you have to write to create all of them using namedtuple from the collections module:

from collections import namedtuple
Creator = namedtuple("Creator", ["first_name", "last_name", "email", "phone_num",
 "address", "city", "country"])
Organization = namedtuple("Organination", ["name", "address", "city", "country"])
BankAccountDetail = namedtuple("BankAccountDetail", ["account_name", "account_num",
 "currency", "bank_name", "branch", "branch_addr"])
Project = namedtuple("Project", ["name", "description", "amount"])
File = namedtuple("File", ["filename", "font_size", "line_height", "orientation"])

dataclasses might also be used to get a similar result.


Code duplication

There is a massive amount of duplicate code in generate_pdf. You could layout the document using a list and string formatting and then iterate over that list to finally write it to a file. Let me give you a sketch of what I mean (Note: the code below is untested):

def generate_pdf(self):
 dt = datetime.now()
 date = dt.date()
 pdf = fpdf.FPDF(format=self.file.orientation)
 pdf.add_page()
 pdf.set_font("Arial", size=self.file.font_size)
 pdf_content = [
 f"Invoice Number #{self.invoice_num}", 
 f"Date Invoiced #{date}",
 # and so on and so forth
 ]
 for line in pdf_content:
 pdf.write(self.file.line_height, line)
 pdf.ln()
 pdf.output(self.file.filename)

The code uses f-string, which are available since Python 3.6. If you are using an older version of Python you will have to use .format instead such as you do in several places already.

There might be even better ways to do this, but I have no specific knowledge about that special library.


Misc

It might be a good idea to have a look at the official Style Guide for Python Code (often just called PEP8) for short. It's a collection of style recommendations and other paradigms that allow you to write compelling and visually appealing Python code.

file in file = File("Invoice.pdf", 12, 5, "letter") is not a good variable name since you overwrite Python's file command with this. At least append a _ to make it file_ or choose another name altogether.

Maybe it would also be worth to have a look at if __name__ == "__main__": to separate the "library part" from the example code.

from datetime import datetime
from collections import namedtuple
import fpdf
Creator = namedtuple("Creator", ["first_name", "last_name", "email", "phone_num",
 "address", "city", "country"])
# all the other code ...
if __name__ == "__main__":
 creator = Creator('Test', 'User', '[email protected]', '099006789',
 'Joans Apartment, 123 Test road', 'Nairobi', 'Kenya')
 organization = Organization('Test Org', 'Ndemi Road Kilimani', 'Nairobi',
 'Kenya')
 bank_detail = BankAccountDetail('Test User', '999999678', 'KES', 'Test Bank',
 'Kenya', 'BRANCH Way, ABC Place')
 file = File("Invoice.pdf", 12, 5, "letter")
 project = Project('Ecommerce site', 'Worked on the ecommerce site', 10.900)
 pdf_inv = PdfInvoice('1393939', creator, organization, project, bank_detail,
 file)
 pdf_inv.generate_pdf()
answered Jun 14, 2019 at 15:46
\$\endgroup\$
1
  • \$\begingroup\$ @AlexV- This is so helpful. I really appreciate it. \$\endgroup\$ Commented Jun 14, 2019 at 20:53
1
\$\begingroup\$

Might want to look at LaTex. It's a lot less code and it IS possible to add comments in, so it's definetly possible to use it as both a template and a way to generate a pdf!

answered Jun 14, 2019 at 21:43
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.