Posts: 29
Threads: 10
Joined: Jun 2018
Hi Python Mentors,
This task is using "Class" with User Input in a sign up form online. The code is running but I would like a code review from anyone. Please be honest and constructive with your feedback as this will really help me learn and improve my current code below. Thank you in advance.
class Studentform:
def __init__(self, fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like):
self.fname = fname
self.lname = lname
self.email = email
self.ccode = ccode
self.mobile = mobile
self.github = github
self.country = country
self.identity = identity
self.want = want
self.codelevel = codelevel
self.subs = subs
self.like = like
@classmethod
def input(cls):
return cls(
input("First Name: \n"),
input("Last Name: \n"),
input("Email: \n"),
int(input("Country code: \n")),
int(input("Phone number: \n")),
input("Github profile: \n"),
input("Country: \n"),
input("Identity (Woman/Non Binary/Man): \n"),
input("I want to (Find my next job/Become a freelancer/Start my own tech startup): \n"),
input("My coding level is (Beginner/Intermediate/Advanced): \n"),
input("I want to subscribe to and join the following groups (Student/Working/Job seeking/Entrepreneur/Mom/Single Mom/Non-Binary/Trans/Retired/Career changer): \n"),
input("I would also like to (Volunteer/Mentor/Hire): \n")
)
user = Studentform.input()[Image: 02_zpspnqlxoos.png]
[Image: 01_zpsw1sjpgpw.png]
Posts: 4,220
Threads: 97
Joined: Sep 2016
I would not name the class method input, rather from_input or something similar. Since it's a method, it won't override the input built-in, but it just makes me cringe. I think it's a good habit never to name things after built-ins, so that you never do it when it could cause a problem.
I would also assign the inputs to some local variables, and then create the class with the local variables. Yeah, you can get user input as a parameter, but it strikes me as a bad idea for some reason. Also, you will want it set up my way if you ever do any validation on the user input.
Posts: 29
Threads: 10
Joined: Jun 2018
(Aug-23-2018, 03:04 PM)ichabod801 Wrote: I would not name the class method input, rather from_input or something similar. Since it's a method, it won't override the input built-in, but it just makes me cringe. I think it's a good habit never to name things after built-ins, so that you never do it when it could cause a problem.
I would also assign the inputs to some local variables, and then create the class with the local variables. Yeah, you can get user input as a parameter, but it strikes me as a bad idea for some reason. Also, you will want it set up my way if you ever do any validation on the user input.
Hi ichabod801, sorry for the late response. Please see my updated code below and let me know if I need to improve it? Thank you
class Studentform:
def __init__(self, fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like):
self.fname = fname
self.lname = lname
self.email = email
self.ccode = ccode
self.mobile = mobile
self.github = github
self.country = country
self.identity = identity
self.want = want
self.codelevel = codelevel
self.subs = subs
self.like = like
@classmethod
def raw_input(cls):
return cls(var1, var2, var3, var4, var5, var6, var7, var8, var9, var10, var11, var12)
var1 = input("First Name: \n")
var2 = input("Last Name: \n")
var3 = input("Email: \n")
var4 = int(input("Country code: \n"))
var5 = int(input("Phone number: \n"))
var6 = input("Github profile: \n")
var7 = input("Country: \n")
var8 = input("Identity (Woman/Non Binary/Man): \n")
var9 = input("I want to (Find my next job/Become a freelancer/Start my own tech startup): \n")
var10 = input("My coding level is (Beginner/Intermediate/Advanced): \n")
var11 = input("I want to subscribe to and join the following groups (Student/Working/Job seeking/Entrepreneur/Mom/Single Mom/Non-Binary/Trans/Retired/Career changer): \n")
var12 = input("I would also like to (Volunteer/Mentor/Hire): \n")
user = Studentform.raw_input()
Posts: 4,220
Threads: 97
Joined: Sep 2016
Now you're using global variables, another problematic choice. What I was thinking of was more like:
@classmethod
def from_input(cls):
fname = input('First Name:\n')
lname = input('Last Name:\n')
email = input('Email:\n')
...
return cls(fname, lname, email, ...)
Posts: 29
Threads: 10
Joined: Jun 2018
(Aug-27-2018, 05:19 PM)ichabod801 Wrote: Now you're using global variables, another problematic choice. What I was thinking of was more like:
@classmethod
def from_input(cls):
fname = input('First Name:\n')
lname = input('Last Name:\n')
email = input('Email:\n')
...
return cls(fname, lname, email, ...)
Thank you for getting back to me, ichabod801. :)
Hope the new structure of my new code is much better this time?
class Studentform:
def __init__(self, fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like):
self.fname = fname
self.lname = lname
self.email = email
self.ccode = ccode
self.mobile = mobile
self.github = github
self.country = country
self.identity = identity
self.want = want
self.codelevel = codelevel
self.subs = subs
self.like = like
@classmethod
def raw_input(cls):
fname = input("First Name: \n")
lname = input("Last Name: \n")
email = input("Email: \n")
ccode = int(input("Country code: \n"))
mobile = int(input("Phone number: \n"))
github = input("Github profile: \n")
country = input("Country: \n")
identity = input("Identity (Woman/Non Binary/Man): \n")
want = input("I want to (Find my next job/Become a freelancer/Start my own tech startup): \n")
codelevel = input("My coding level is (Beginner/Intermediate/Advanced): \n")
subs = input("I want to subscribe to and join the following groups (Student/Working/Job seeking/Entrepreneur/Mom/Single Mom/Non-Binary/Trans/Retired/Career changer): \n")
like = input("I would also like to (Volunteer/Mentor/Hire): \n")
return cls(fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like)
user = Studentform.raw_input()The program is running perfectly in my terminal.
[Image: a_zpskzee2lad.png]
Posts: 4,220
Threads: 97
Joined: Sep 2016
That's what I was thinking, yes.
Posts: 29
Threads: 10
Joined: Jun 2018
(Aug-27-2018, 07:54 PM)ichabod801 Wrote: That's what I was thinking, yes.
I appreciate you, ichabod801.
Thank you so much!
Posts: 4,220
Threads: 97
Joined: Sep 2016
To be honest, whenever I see something like your code, I think "how can I loop this?" That is, whenever a programmer is doing something over and over again, they should think about putting it in a loop. At least, if it's all in the same place like your code is. If you are doing the same thing in different places, you should think about making a function, and calling that function from those different places.
So, in your code you are asking questions and assigning them to parameters in an instance creation. Some of them you convert to int. How could we make that into a loop?
class Studentform:
questions = {'fname': "First Name: \n", 'lname': "Last Name: \n", 'email': "Email: \n",
'ccode': "Country code: \n", 'mobile': "Phone number: \n", 'github': "Github profile: \n",
'country': "Country: \n", 'identity': "Identity (Woman/Non Binary/Man): \n",
'want': "I want to (Find my next job/Become a freelancer/Start my own tech startup): \n",
'codelevel': "My coding level is (Beginner/Intermediate/Advanced): \n",
'subs': "I want to subscribe to and join the following groups (Student/Working/Job seeking/Entrepreneur/Mom/Single Mom/Non-Binary/Trans/Retired/Career changer): \n",
'like': "I would also like to (Volunteer/Mentor/Hire): \n"}
to_int = ['ccode', 'mobile']
def __init__(self, fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like):
self.fname = fname
self.lname = lname
self.email = email
self.ccode = ccode
self.mobile = mobile
self.github = github
self.country = country
self.identity = identity
self.want = want
self.codelevel = codelevel
self.subs = subs
self.like = like
@classmethod
def raw_input(cls):
kwargs = {}
for parameter, question in self.questions.items():
kwargs[parameter] = input(question)
if parameter in self.to_int:
kwargs[parameter] = int(kwargs[parameter])
return cls(**kwargs)
user = Studentform.raw_input()This makes use of the ** operator. In parameter lists, it turns a dictionary into key word parameters, using the key/value pairs of the dictionary as parameter/value pairs.
Here, this technique is kind of borderline useful. My code takes 13 lines where yours took 12. However, in more complicated situations it can clean up your code an incredible amount. This code is easier to update. Say you decided later on all responses should be stored in upper case. In my code you would only have to change one line. In your code, you would have to change 10.
Posts: 29
Threads: 10
Joined: Jun 2018
(Aug-28-2018, 11:03 AM)vanicci Wrote: [quote='ichabod801' pid='56597' dateline='1535399654']
That's what I was thinking, yes.
Thank you very much for following up and for suggesting a more efficient structure of coding the task. You're sooo kind! :)
I ran the new code that you provided above and I encountered an error [ see photos below] BUT I changed it a little bit and it is now working in order. We did it! :)
[Image: 01_zpsnhejgbtx.png]
[Image: 02_zpsleexb95r.png]
NEW CODE
class Studentform:
questions = {'fname': "First Name: \n", 'lname': "Last Name: \n", 'email': "Email: \n",
'ccode': "Country code: \n", 'mobile': "Phone number: \n", 'github': "Github profile: \n",
'country': "Country: \n", 'identity': "Identity (Woman/Non Binary/Man): \n",
'want': "I want to (Find my next job/Become a freelancer/Start my own tech startup): \n",
'codelevel': "My coding level is (Beginner/Intermediate/Advanced): \n",
'subs': "I want to subscribe to and join the following groups (Student/Working/Job seeking/Entrepreneur/Mom/Single Mom/Non-Binary/Trans/Retired/Career changer): \n",
'like': "I would also like to (Volunteer/Mentor/Hire): \n"}
to_int = ['ccode', 'mobile']
def __init__(self, fname, lname, email, ccode, mobile, github, country, identity, want, codelevel, subs, like):
self.fname = fname
self.lname = lname
self.email = email
self.ccode = ccode
self.mobile = mobile
self.github = github
self.country = country
self.identity = identity
self.want = want
self.codelevel = codelevel
self.subs = subs
self.like = like
@classmethod
def raw_input(self, **kwargs):
kwargs = {}
for parameter, question in self.questions.items():
kwargs[parameter] = input(question)
if parameter in self.to_int:
kwargs[parameter] = int(kwargs[parameter])
user = Studentform.raw_input()
|
