While your sendemail class is small, it does a lot of stuff. It is also not that friendly to use, because you have to create the sendemail object and then set all the parameters with methods. For me an ideal interface would look something like:
Email(
Subject="sub",
Body="body",
From="from",
To=["to"],
Attachments=[file1, file2],
)
.send()
You could then have separate file readers which would convert file path to email part:
class Attachment:
def __init__(self, path):
self.path = path
def file(self):
file = self.__read_file(self.path)
return convert_to_part(file)
def zip(self):
file = self.__read_file(path)
zip_buffer = self.__zip_file(file)
return convert_to_part(zip_buffer)
def __convert_to_attachment(buffer, filename):
part = MIMEBase('application', "octet-stream")
part.set_payload(buffer)
Encoders.encode_base64(part)
part.add_header('Content-Disposition', 'attachment; filename="%s.zip"'%(filename))
return part
def __read_file(self):
return open(self.path, "rb").read()
def __zip_file(self, file):
filename=os.path.basename(file)
zipped_buffer = io.BytesIO()
zf = zipfile.ZipFile(zipped_buffer,"w", zipfile.ZIP_DEFLATED)
zf.write(file, filename)
zf.close()
zipped_buffer.seek(0)
return zipped_buffer
I am not sure if I refactored everything correctly, but the idea is to use it like:
Email(
...
Attachments=[Attachment('path').zip()]
)
.send()
You could modify the Email to have a builder style attaching:
class Email:
...
attach(self, attachment):
self.Attachments.append(attachment)
...
Email(...)
.attach(Attachment('path').zip())
.attach(Attachment('path').file())
.send()
This leaves you with the following Email class:
class Email(object):
def __init__(self, From, To, Subject, Body, **kwargs):
self.subject = Subject
self.body = Body
self.mail_from = From
self.mail_to = To
self.attachments = kwargs.get('Attachments', [])
def __build_message(self):
message = MIMEMultipart()
message["From"] = self.mail_from
message["To"] = self.mail_to
message["Subject"] = self.subject
message.attach(MIMEText(self.body, "plain"))
for part in self.attachments:
message.attach(part)
return message.as_string()
def send(self):
message = self.__build_message()
server = smtplib.SMTP('localhost')
server.sendmail(self.mail_from, self.mail_to, message)
server.quit()
As you can see, the class Email is now very simple. So simple in fact that it could easily be converted to a function. I come from Java, so objects do not bother me, but for the sake of being principled:
def build_message(From, To, Subject, Body, **kwargs):
message = MIMEMultipart()
message["From"] = From,
message["To"] = To
message["Subject"] = Subject
message.attach(MIMEText(Body, "plain"))
for part in self.attachments:
message.attach(part)
return message.as_string()
def send_email(From, To, Subject, Body, **kwargs):
message = build_message(From, To, Subject, Body, **kwargs)
server = smtplib.SMTP('localhost')
server.sendmail(self.mail_from, self.mail_to, message)
server.quit()
You could do a similar thing with the Attachment class. So instead of Attachment.file(path) you would make a file_attachment(path) and zip_attachment(path).
Edit:
To make the code easier to use for the OP, I propose the following utility function(s):
def build_file_attachments(paths):
return [Attachment(path).file() for path in paths]
def build_zip_attachments(paths):
return [Attachment(path).zip() for path in paths]
def build_attachments(file_attachment_paths, zip_attachment_paths):
file_attachments = build_file_attachments(file_attachment_paths)
zip_attachments = build_zip_attachments(zip_attachment_paths)
return file_attachments + zip_attachments
def send_email_with_attachments(
From,
To,
Subject,
Body,
file_attachment_paths=[],
zip_attachment_paths=[]
):
attachments = build_attachments(file_attachment_paths, zip_attachment_paths)
send_email(From, To, Subject, Body, Attachments=attachments)
Now you would use it the same as your original one, but without the setters and the "spaghetti" you had in your send() method before.
send_email_with_attachments(
From="[email protected]",
To="[email protected]",
Subject="Very important!",
Body="I wanted to show you something",
file_attachment_paths=["important-thing-1.jpg"],
zip_attachment_paths=["important-thing-2/dir"]
)
If you still find this confusing and hard to use, you can do something similar to what you had originally:
class Email:
def __init__(self, from, to, subject):
self.__from = from
self.__to = to
self.__subject = subject
self.__body = ''
self.__zips = []
self.__files = []
def sender(self, from):
self.__from = from
return self
def recipient(self, to):
self.__to = to
return self
def subject(self, subject):
self.__subject = subject
return self
def message(self, body):
self.__body = body
return self
def add_new_line(self, line=''):
self.__body = "%s\r\n%s"%(self.body, line)
return self
def add_zip(self, path):
self.__zips.append(path)
return self
def add_file(self, path):
self.__files.append(path)
return self
def send():
send_email_with_attachments(self.from, self.to, self.subject, self.body, self.files, self.zips)
You can use this like so:
email = Email("[email protected]", "[email protected]", "Not so important")
email.message('Hi,')
email.add_new_line()
email.add_new_line('actually it is important. Check this out:')
email.add_new_line()
email.add_file('actually-not-important.pdf')
email.add_zip('./important')
email.send()
Which is basically the same as:
Email("[email protected]", "[email protected]", "Not so important")
.message('Hi,')
.add_new_line()
.add_new_line('actually it is important. Check this out:')
.add_new_line()
.add_file('actually-not-important.pdf')
.add_zip('./important')
.send()
Choose whichever style you prefer :)
Email class is now just there to remember the parameters, it has no idea, what needs to be done to actually send the email. The send method now passes the parameters into send_email_with_attachments(), which handles the sending... Kind of...
It also has no idea how to actually send the email and that is ok, it just delegates the parameters into the functions that know how to do this stuff, which makes it a lot easier to understand. The point of all this is that you want to move lower level stuff (like reading files, zipping, making multipart message, etc.) as far away as possible, so you are able to program at a higher level (closer to natural language).
The side effect of this is also that you will end up with a ton of reusable code, which you will then be able to use to assemble the higher level code from.
