Generate code from full name and index number

Question

Does this script look well-written and formulated?

'''
This is for a practice
Get the last letter from first_name, 2nd and 3rd letters from last name
First 5 numbers from index number.
And generate a a unique code.
'''
import sys
first_name = 'first_name'
last_name = 'last_name'
index_number = str('02001201525') # Converting number to str so it can concat
def from_index(index_number):
 for n in index_number.split():
 return n[0:5]
def generate_code(first_name, last_name):
 letters_from_first_name = first_name[-1]
 letters_from_last_name = last_name[0:2]
 n_from_index = from_index(index_number)
 code = letters_from_first_name + letters_from_last_name + n_from_index
 return code # To the world
 print generate_code(first_name, last_name);
 sys.exit() # Is importing the sys module important as i am not iterating

• 301_Moved_Permanently: Is it just me or does the code perform something different than what the docstring is claiming?
• SuperBiasedMan: @MathiasEttinger It does match the docstring, but it's a bit obfuscated with unnecessary code and unexplained default (I assume) values.
• 301_Moved_Permanently: @SuperBiasedMan last_name[0:2] doesn't seem to be "second and third letters from last name"
• SuperBiasedMan: @MathiasEttinger Ah, I missed that thanks! I had assumed you meant because from_index is odd behaviour. I noted it in my answer now.
• SuperBiasedMan: Hi @blakroku! I noticed you tried to edit a new revision into this post. On Code Review we encourage people to make a new question if they'd like to have their new version of a script reviewed rather than editing the original post. Please see what you may and may not do after receiving answers .

Answer 1

Your code is quite confusing, are you new to Python? You seem to have some misunderstandings about how it works. First of all, no you don't need sys. Python scripts end automatically when they reach the end. You only need to call sys.exit() if you want to end a script in the middle of it for any reason.

Next, you're calling str on a string that's already a string. Based on the other variables you've defined this might be a placeholder, but it's confusing so it's worth saying that you're using placheolder values. I would also put it in as a number, except that you start the number with 0. An int can't have 0 as its first digit, so either you should be getting that number as a string (removing the need for str()) or there's something else going on here. Also I would suggest that you put the comment on a separate line, to be more readable.

# Converting number to str so it can concat
index_number = str('02001201525')

The from_index function is confusing. The name doesn't tell you what it does. It runs split but there's no need to if you just need the first 5 characters. It also probably shouldn't be a function. It has a very specific seeming use, takes no useful parameters and in the end just returns [0:5] on the string that's passed into it. It's easier to just do this:

index_number[0:5]

The only difference between that and your function is that your function would cut the result short if there was white space in the number, but that doesn't sound like it's what you want.

As for your generate_code, it does work for the most part, but is again a lot of lines to do something relatively simple. It also doesn't take index_number as a parameter even though it really should. I'd actually reduce it to one line, like this:

def generate_code(first_name, last_name, index_number):
 return first_name[-1] + last_name[2:4] + index_number[0:5]

Note that you also put in the wrong parameters to get the second and third parameters of last_name. I'm not sure if it was just a typo or misunderstanding, but I'll explain slicing if you need me to. Just comment and I can write up a bit about it.

Answer 2

Your code is much, much harder and longer than it should be:

• sys is not necessary
• from_index is absolutely not needed, and the loop in it is useless
• You assign way too many variables while you could use the values directly.
• The constants at the top should be UPPERCASE
• You forgot giving the index_number input to the function.

generate code can be written in one line while being more readable than your version.