14
\$\begingroup\$

After a few pages reading Learning Python Hard Way, I've decided to try to make a simple cm-to-inch calculator. I am very excited about this achievement, and I want someone to check my code. It works well, but i just need someone to confirm everything is written well.

# cm to inch calculator
inch = 2.54 #centimeters
centimeters = int(input("Enter a number of centimeters"))
print "Entered number of %s centimeters is equal to %s inches" % (centimeters , centimeters / inch)
\$\endgroup\$
20
\$\begingroup\$

Great work so far! And good on you for seeking review. In general, your code is pretty good, and works correctly. As you've asked, though, here are some suggestions for improvement.


The assignment:

inch = 2.54 #centimeters

is potentially confusing; instead, you could make it clear that it is a ratio:

CM_PER_INCH = 2.54

Note the UPPERCASE_WITH_UNDERSCORES name for a constant, per the style guide.


centimeters = int(input("Enter a number of centimeters"))

There is no reason to limit the user to int input - why not allow float? Then they can enter and convert e.g. 4.2cm.

Also, this will fail with input that can't be converted to an integer (what if the user types seven?) - have a look at this SO community wiki if you want to add validation.


print "Entered number of %s centimeters is equal to %s inches" % (centimeters , centimeters / inch)

This line is too long (see the style guide again). Also, % formatting is a little old-fashioned, compared to str.format. I would split this into two lines:

template = '{.2f}cm is equal to {.2f}"'
print template.format(centimeters, centimeters / CM_PER_INCH)

Or use line continuation:

print '{.2f}cm is equal to {.2f}"'.format(
    centimeters,
    centimeters / CM_PER_INCH
)

In use:

>>> print template.format(float(centimeters), centimeters / CM_PER_INCH)
5.00cm is equal to 1.97"

If you want another challenge, have a go at implementing the reverse calculation, and allowing the user to choose which way to convert.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ A lot new things to learn, thank you so much! Although i have no idea what "template" is or "{.2f}", i guess i haven't reached that lectures yet. :D \$\endgroup\$ – nikola1970 Feb 15 '15 at 21:18
  • 3
    \$\begingroup\$ What do you mean "what "template" is"? It's just a string, like your "Entered number of %s centimeters is equal to %s inches". In terms of the {.2f}, see docs.python.org/2/library/string.html#formatstrings \$\endgroup\$ – jonrsharpe Feb 15 '15 at 21:20
  • 1
    \$\begingroup\$ Oh yes, it's just a string, my bad, thanks! :) \$\endgroup\$ – nikola1970 Feb 15 '15 at 21:23
9
\$\begingroup\$

Your code looks ok and probably does the job properly :-)

Style

However, it is not quite perfect. Python has a code style called PEP8 and is definitly worth a read. Among other things, it says :

Constants Constants are usually defined on a module level and written in all capital letters with underscores separating words. Examples include MAX_OVERFLOW and TOTAL .

Here, inch definitly looks like a constant so INCH would be better.

Also, not really important but you have an "extraneous whitespace" in (centimeters , centimeters.

Functions

Also, instead of having your conversion logic hidden in your logic handling input/output (with print), you might want to extract in in a simple function.

Organisation

Another to know if you are beginning with Python is that a good thing to do is to have your definitions of constants/functions/classes/etc in a file in such a way that you can re-use it later on (via imports mechanisms). If you don't want your logic "actually doing something" to mess with everything when you import your code, the technic is to use an if __name__ == "__main__": guard.

Naming

Naming is quite important in programming. Here centimeters in top of being long does not convey much meaning. I reckon nb_cm gives more information. Similarly, NB_CM_IN_INCH is better than INCH.

User interface

It could be a good idea to handle float values on top of integers.

Python 3 (and notes about input)

Final thing before I forget, from the way you have used print, I guess you are using Python 2. If you have no special reason to do so, you might want to start using Python 3 instead. You'll find a few things slightly different but you'll be better off learning directly on the latest versions so you have the good habits and nothing to un-learn.

However, I have to point out that input is "slightly" different on the 2 versions of Python. input from Python 3 corresponds to raw_input while input from Python 2 can be simulated in Python 3 with eval(input()).

Indeed, as you might or might not know, what you were doing is doing more that merely asking a string from the user, it was asking for Python code and handling it as such.

For instance,

Enter a number of centimeters 2*3

Entered number of 6 centimeters is equal to 2.36220472441 inches

See how the number was evaluated. It looks good in theory or in you really know what you are doing. In most cases, using eval directly or indirectly is considered a bad practice.

Final code

Here's my version of the code (works in Python 2 but behavior is slightly different - cf note above).

NB_CM_IN_INCH = 2.54

def convert_cm_in_inch(val):
    """You can put docstrings here :)."""
    return val / NB_CM_IN_INCH

def convert_inch_in_cm(val):
    """And here :D."""
    return val * NB_CM_IN_INCH

if __name__ == "__main__":
    nb_cm = float(input("Enter a number of centimeters: "))
    print("Entered number of %s centimeters is equal to %s inches" % (nb_cm, convert_cm_in_inch(nb_cm)))

It seems like I have said many things so I want you not to take it as criticism of your good but as advices if you keep on programming in Python.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I guess i haven't learned "def" thing yet, i just passed strings and math operations so far and coz of that i have no idea what "def" is doing, but i hope i will learn that soon. :D Thanks a ton friend, you put so much effort to help! \$\endgroup\$ – nikola1970 Feb 15 '15 at 21:20

Not the answer you're looking for? Browse other questions tagged or ask your own question.