good overall function purpose commenting and function naming. By reading the first 2 lines of any of the functions I already have a good idea what is about to go down.
length
Your code density is pretty sparse. user_instance could be rewritten as
def user_instance():
'''Creates boto3 session and ec2 instance object from user input'''
# get user input <-
iam_user = input("Enter your Identity Access Management user profile: ")
clear()
ec2_id = input("Enter the EC2 instance Id you would like to look up: ")
clear()
# get ec2 instance <-
return boto3.session.Session(profile_name=iam_user).resource('ec2').Instance(id=ec2_id)
In your code this function stretches out for nearly 30 lines. This can cause a couple issues.
the code is so commented it goes all the way around the bend and becomes difficult to read again.
the actual functionality has been spread so thinly the code no longer makes too much sense on its own. For example, why do we grab ec2_resource? We have to go down 8 lines to see it used again and then only to return a child.
the code being spread this thin makes the code longer.
The more you can see on a single screen, the better. Scrolling around and losing place is an annoyance and will drive you to split out a file.
Clutter gets pushed up the ladder. Splitting out files leads to adding subfolders sooner which can lead to a messy project.
White space loses meaning
Here you place about 1 empty line between each line and then 2 or 3 empty lines between functions. The presence of whitespace when you are used to its absence is more dramatic than this. I will bet you noticed the next line before getting to this segment.
I would suggest no empty lines between related lines of code, 1 empty line between segments in code, and then 1-2 empty lines between functions. Functions also give notice to themselves because they have colored key words and are all the way left.
It is technically wasting memory and processing power
probably the least important reason, as the effect is negligible in most circumstances and because compiled languages will optimize the issues out anways.
each one of the variables you define that are then never used are actually being generated in memory. Because python is interpretted, it has to read in 78 lines when this file could maybe be compressed down to 30 lines.
Even the comments I pointed out with arrows are technically not necessary since the code is pretty self explanatory.
I still like to write little headers for code segments though, so I would leave them in, but thats a style thing.
string extraction
This is not an important suggestion, but something to consider when thinking of things to do when scaling an application.
The basic idea is to remove strings from your code and instead use constants. For user facing strings you would place all user facing strings into a single file. The purpose is so that in editor mode you can ignore code. And additionally, (if you are ever releasing in multiple languages) so you can have this one file translated and you're done.
For internal strings such as running on line 58. These would not go into the strings file previously mentioned. They would go somewhere depending on their scope; likely at the top of the file, or in a globals file. The purpose is to take advantage of your IDE's autocomplete and not misspell things. Also, if ever it turns out the spec changes, you can change this single string and all of your code begins working again. No diving through your code.
/ec2_instance_menu
You have chosen good variables to define. Every variable is used and important.
major tradition ouch
This is your main file. It should look special. When I opened your repo, it was not clear to me which file was the main. It does not need to be named Main but generally the main is called out with a distinctly casual name. Some I have seen are
main
program
index (generally only web projects)
[project name] ( this is probably the choice you were going for, but your project name is rather technical sounding, so it doesn't really stand out compared to the other files. One project I follow is pokemon showdown, their main is pokemon-showdown which does distinguish itself)
meaning split between files
I am forced to determine the meaning of choice 1,2, 3 from the response you have written for it. This means likely I would have to open menu_options in order to properly develop this file. This means we need to either, recollapse menu_options back into this file or find a way to make the meaning more clear. I would honestly suggest at this size, to just collapse menu_options back into this file, but since we are practicing for larger projects I will suggest defining an enum.
in menu_options define a user options enum
from enum import Enum
class User_Options(Enum):
START_INSTANCE = 1
STOP_INSTANCE = 2
EXIT_PROGRAM = 3
Now in ec2_instance_menu, you can import this enum along with main_menu and refer to things in comparison with this enum. Now your code can look like
if user_choice == User_Options.EXIT_PROGRAM:
quit()
Additionally you can now reorder and renumber the meanings of enum and every file referencing it will automatically be updated, so maintenance and growth will be simpler.
/menu_options
not much to say about this one. Hopefully this file grows into a strong file someday.
/helper_functions
the term helper function is a very common term, but having a file devoted to them is pretty uncommon. Generally only this case in informal and/or small projects. It implies that functions of unrelated use will be dropped into it and thats generally not a good idea for something you hope to scale. So a name more targetted at a unifying reason is suggested. Maybe user_input.
maximize laziness.
one of my core goals while programming is to be as actively lazy as possible. Some parts of that are
Never copy code
Maximize code reuse
Your function clear is pretty good, I notice that you use it a lot throughout you code. You've probably saved something like 20 lines for it.
I also notice that you almost always use it directly after asking the user for input. So, you can go even further with your laziness.
Now you can replace every time you ask for user input and then clear the screen with this new function. Additionally, this allows you to standardize how your user interacts with your program in one place. You can change this as much as you want, but the user experience will always be consistent since there will not be an outlier where you forgot to put clear() after asking for input.
Now that we've got standard interface we can start doing fancier stuff. Like adding interesting information to the input line such as aws console path and status (idk how aws console works, just making stuff up) and then maybe replace clear with a couple new lines.
/instance_session
good overall function purpose commenting and function naming. By reading the first 2 lines of any of the functions I already have a good idea what is about to go down.
length
Your code density is pretty sparse.
user_instance
could be rewritten asIn your code this function stretches out for nearly 30 lines. This can cause a couple issues.
ec2_resource
? We have to go down 8 lines to see it used again and then only to return a child.string extraction
This is not an important suggestion, but something to consider when thinking of things to do when scaling an application. The basic idea is to remove strings from your code and instead use constants.
For user facing strings you would place all user facing strings into a single file. The purpose is so that in editor mode you can ignore code. And additionally, (if you are ever releasing in multiple languages) so you can have this one file translated and you're done. For internal strings such as
running
on line 58. These would not go into the strings file previously mentioned. They would go somewhere depending on their scope; likely at the top of the file, or in a globals file. The purpose is to take advantage of your IDE's autocomplete and not misspell things. Also, if ever it turns out the spec changes, you can change this single string and all of your code begins working again. No diving through your code./ec2_instance_menu
You have chosen good variables to define. Every variable is used and important.
major tradition ouch
This is your main file. It should look special. When I opened your repo, it was not clear to me which file was the main. It does not need to be named
Main
but generally the main is called out with a distinctly casual name. Some I have seen arepokemon-showdown
which does distinguish itself)meaning split between files
I am forced to determine the meaning of choice 1,2, 3 from the response you have written for it. This means likely I would have to open
menu_options
in order to properly develop this file. This means we need to either, recollapsemenu_options
back into this file or find a way to make the meaning more clear. I would honestly suggest at this size, to just collapse menu_options back into this file, but since we are practicing for larger projects I will suggest defining an enum. in menu_options define a user options enumNow in
ec2_instance_menu
, you can import this enum along with main_menu and refer to things in comparison with this enum. Now your code can look likeAdditionally you can now reorder and renumber the meanings of enum and every file referencing it will automatically be updated, so maintenance and growth will be simpler.
/menu_options
not much to say about this one. Hopefully this file grows into a strong file someday.
/helper_functions
the term helper function is a very common term, but having a file devoted to them is pretty uncommon. Generally only this case in informal and/or small projects. It implies that functions of unrelated use will be dropped into it and thats generally not a good idea for something you hope to scale. So a name more targetted at a unifying reason is suggested. Maybe
user_input
.maximize laziness.
one of my core goals while programming is to be as actively lazy as possible. Some parts of that are
Your function
clear
is pretty good, I notice that you use it a lot throughout you code. You've probably saved something like 20 lines for it. I also notice that you almost always use it directly after asking the user for input. So, you can go even further with your laziness.Now you can replace every time you ask for user input and then clear the screen with this new function. Additionally, this allows you to standardize how your user interacts with your program in one place. You can change this as much as you want, but the user experience will always be consistent since there will not be an outlier where you forgot to put
clear()
after asking for input.Now that we've got standard interface we can start doing fancier stuff. Like adding interesting information to the input line such as aws console path and status (idk how aws console works, just making stuff up) and then maybe replace clear with a couple new lines.
the end.
I tried my best to roast this. I think its actually pretty good.