5
\$\begingroup\$

Just started learning python and I'm solving some beginner problems. I wrote a solution for a question without any edge cases like user not entering anything, entering numbers etc.:

Question: Write a program to print initial letter of an entered name?

Expected output: If the user inputs,

  1. Ram Chandra Giri -> R.C.Giri

  2. Ram chandra Giri -> R.C.Giri

My solution works as expected but I would like insight on whether I am following best practice or not and if there is better and easier way to do it. Question is from Class 10 book for students to learn Q-BASIC.

My solution:

def initial_of_name(c):
    word_list = c.split(" ")
    i = (len(word_list) - 1)
    new_list = []
    for w in range(0, i):  # this loop leaves last name and creates a new list.
        word = word_list[w]
        word = word[0:1].upper()  # sliced word into initial letter
        new_list.append(word)
    new_list.append(word_list[-1])  # added last name to the list before joining.
    initial = '.'.join(new_list)
    print("The initial of your name is {0}.".format(initial))
    return


name = input("Please enter your name?\n>")
initial_of_name(name)
\$\endgroup\$
11
\$\begingroup\$

In general, I think that's a great start - it's consistently formatted and sensibly structured.


def initial_of_name(c):

c isn't a great name for a parameter, it tells the reader nothing about what the function is expecting to get. It might also be helpful to add a docstring to provide a bit more information.


word_list = c.split(" ")

If you're splitting on spaces, note that you can just use .split(), which will also have the effect of counting consecutive whitespace as a single space. This is helpful if someone accidentally types My Name instead of My Name; with the current formulation you'd get ['My', '', 'Name'].


i = (len(word_list) - 1)
...
for w in range(0, i):
    word = word_list[w]

Iterating over a list by index in Python is a bit of an anti-pattern, there are lots of better tools for dealing with iteration. In this case, you could iterate over a slice of all but the last item in the list:

for word in word_list[:-1]:

Talking of slicing:

word = word[0:1].upper()

You don't need the 0, that's the default start to a slice, so it could be word[:1]. But what you want is the first character, which is perhaps more clearly expressed as word[0].


new_list = []
for ...:
    ...
    new_list.append(...)

Creating an empty list, iterating and appending is often not the most efficient way to build a new list. Instead, consider a list comprehension (if this is unfamiliar to you, see e.g. Python for-in loop preceded by a variable).


print("The initial of your name is {0}.".format(initial))

In general, I wouldn't use print in a function like this. This is part of separation of concerns; building the appropriate string and displaying the result to the user are two completely different tasks, so they shouldn't be in the same function. Additionally, printing rather than returning the result makes it harder to test the function.


With all of that considered, here's how I would implement the same requirements.

def initial_of_name(name):
    """Converts a name to initials and surname.

    Ensures all initials are capitalised, even if the
    first names aren't.

    Examples:

      >>> initial_of_name('Ram Chandra Giri')
      'R.C.Giri'
      >>> initial_of_name('Ram chandra Giri')
      'R.C.Giri'

    """
    parts = name.split()
    initials = [part[0].upper() for part in parts[:-1]]
    return '.'.join(initials + [parts[-1]])


if __name__ == '__main__':
    name = input("Please enter your name?\n>")
    initial = initial_of_name(name)
    print("The initial of your name is {0}.".format(initial))

If the last part is unfamiliar, see What does if __name__ == "__main__": do?


Note that including examples in the docstring in the form of an interactive Python session not only gives the reader a good idea of exactly how to use the function, but also allows doctest to validate the code and examples for you. If you save the above into a file initial.py:

$ python -m doctest initial.py --verbose
Trying:
    initial_of_name('Ram Chandra Giri')
Expecting:
    'R.C.Giri'
ok
Trying:
    initial_of_name('Ram chandra Giri')
Expecting:
    'R.C.Giri'
ok
1 items had no tests:
    initial
1 items passed all tests:
   2 tests in initial.initial_of_name
2 tests in 2 items.
2 passed and 0 failed.
Test passed.
\$\endgroup\$
  • 1
    \$\begingroup\$ Seems to me that I need to learn a lot of things. Well thank you sir for going over this in such details. \$\endgroup\$ – Ram Chandra Giri Oct 7 '17 at 9:25
4
\$\begingroup\$
  • Choose better names for your variables. Sure, this is a short program, you can remember what w means, but in larger programs you can not.
  • I run your program and forgot to write my last name in capital letter, so your program gave me back my last name not capitalized. You can improve this element even if it is not that bad.
  • In general, it is good to wrap your main functionality in a separate function called main().
  • As the first answer mentioned, you can use split() instead of split("\n") or split("") (documentation)

Given these elements, here is my suggestion:

  • When the user types his name, capitalize each first letter of his names.
  • I then split the full name into a list of names
  • If this list contains only one name, it means the user has written only his first name (or one of his names), in this case, just return what is given with the first letter capitalized in the first step.
  • If the user types his first, middle and last names, then create a separate list for the first names, loop over the list and pick only the first letters: initials = '.'.join(name[0] for name in first_names).
  • Finally, return the initials and with the last name: return initials + '. ' + last_name:

    def initials_of_name(full_name):
        whole_name = full_name.title()
        list_of_names = whole_name.split()
        last_name = list_of_names[-1]
        number_of_names = len(list_of_names)
        if(number_of_names == 1):
            return last_name
        else:
            first_names = list_of_names[:-1]
            initials = '.'.join(name[0] for name in first_names)
            return initials + '. ' + last_name    
    
    def main():
       full_name = input('Please enter your full name:\n')
       print(initials_of_name(full_name))
    
    if __name__ == '__main__':
        main()
    
\$\endgroup\$
  • \$\begingroup\$ Need to learn more about other thing but will start from wrapping main functionality. ++ \$\endgroup\$ – Ram Chandra Giri Oct 7 '17 at 9:42
  • 1
    \$\begingroup\$ Fell free to ask if something in my answer is not clear \$\endgroup\$ – Billal Begueradj Oct 7 '17 at 9:43
  • 1
    \$\begingroup\$ Sir, is this a typo return initials + '. ' + last_name there is a space after . in middle that's why output is slightly different. Other than that nothing as of now, will ask questions after learning more. Thank you. \$\endgroup\$ – Ram Chandra Giri Oct 7 '17 at 9:51
  • 1
    \$\begingroup\$ You understood it right: the '. ' adds a point and a space to separate the last initial letter from the last name. The space is just to make the output pretty looking @RamchandraGiri \$\endgroup\$ – Billal Begueradj Oct 7 '17 at 9:53
  • \$\begingroup\$ If you do not want the space to appear between the last initial letter and the last name, then just replace '. ' by '.' @RamchandraGiri \$\endgroup\$ – Billal Begueradj Oct 7 '17 at 9:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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