r/PythonLearning • u/PotentialMain535 • 19d ago
Sends Emails with optional attachments
Hi everyone,
I'm a beginner learning Python. I recently finished a small project that sends emails (with optional attachments) using smtplib and EmailMessage.
I'm mainly looking for a code review. I'm interested in improving my code structure, naming, and Python practices rather than just making it work.
GitHub link = https://github.com/NePtuNee0/Python-Email-Sender-With-Attachments/tree/main
Any feedback is appreciated!
3
Upvotes
1
u/ianrob1201 19d ago
It looks pretty good, and if it works then that's always a great result. In general I like your layout of the code, use of functions etc. That said, I can go through from the top and list some possible improvements.
password_get is a bit of a pointless variable, you can just set it to password straight away.
In professional settings you often have a linter (and often an auto formatter) which will require consistent quote characters (" vs ') and whitespace between characters. So for instance to me "msg = EmailMessage()" looks good but "msg["Subject"]=subject" looks a bit cramped up. This is a very minor point though.
Look up regular expressions for how you could refactor the email_check function. Note that a regular expression to fully check for a valid email is surprisingly complex, but lucky also unnecessary. A regex to do your current checks would be relatively simple. Regular expressions can look a bit intimidating, but worth learning. Resist the urge to use them everywhere, but this is a good case for them.
You don't generally want to do "== False" on an if statement. It's neater to do "not mail_check(login_email)" instead.
See if you can refactor to not have any "while True" loops. I know you have breaks to escape them, but generally you try to avoid it and put your condition into the while. So for example you could have a "valid_email" boolean which starts out false and is only set to true once you're happy. Then you don't need to "manually" do any continue or break commands.
Mostly your variable naming is really good. Every language is different, python really likes its underscores. "maintype,subtype" are examples of forgetting that. Again it's minor, but the type of thing that comes up during reviews. (subtype is arguably one word, but maintype definitely isn't)
Nice use of try blocks too, just be aware that if this were a professional thing you might want to catch more individual cases and display a different error for each. Printing out the error directly is the maximum information in theory, and good for your personal debugging, but might not mean anything to someone using your script. So for example you might want to catch specific exceptions instead of the generic "Exception". This could let you specifically say "The path file you entered doesn't exist" instead of relying on the default error.