3
\$\begingroup\$

Completed A challenge on HackerRank: Develop a game where a given string is split into all possible substrings and then depending on the beginning of the substring (vowel or consonant) determines which player (Kevin or Stuart) received a point. Hoping I could get advice on neatness, anything I did wrong/needs to be done better.

'''
https://www.hackerrank.com/challenges/the-minion-game/problem

Problem Statement: Both players are given the same string, s
Both players have to make substrings using the letters of the string s
Stuart has to make words starting with consonants.
Kevin has to make words starting with vowels. 
The game ends when both players have made all possible substrings. 

output format: Print one line: the name of the winner and their score separated by a space. 
If the game is a draw, print Draw.

Created on Jan 13, 2018

@author: Anonymous3.1415
'''

#splits the given string
def get_sub_strings(given_string):
    string_leangth = len(given_string)
    return [given_string[i:j+1] for i in xrange(string_leangth) for j in xrange(i, string_leangth)]

def minion_game(given_string):
    split_string = get_sub_strings(given_string)
    vowels = 0
    consonants = 0
    for i in xrange(len(split_string)):
        if split_string[i][0] in "AEIOU":
            vowels += 1
        else:
            consonants += 1

    if vowels > consonants:
        print("Kevin %d") % (vowels)
    elif vowels == consonants:
        print("draw")
    else:
        print("Stuart %d") % (consonants)

    return

given_string = 'BANANA'
minion_game(given_string)
\$\endgroup\$
4
\$\begingroup\$
#splits the given string
def get_sub_strings(given_string):
    string_leangth = len(given_string)
    return [given_string[i:j+1] for i in xrange(string_leangth) for j in xrange(i, string_leangth)]

That comment adds no information to the method name. The method name is already very explicit. The comment rather even adds wrong information, because you don't actually split the string.

You could consider yielding the return values, so that you can iterate over them without needing the complete list in memory.


    split_string = get_sub_strings(given_string)
    vowels = 0
    consonants = 0
    for i in xrange(len(split_string)):
        if split_string[i][0] in "AEIOU":
            vowels += 1
        else:
            consonants += 1

The variable name "split_string" lies about its contents. It contains the substrings, not a split string. Saving the result of get_sub_strings in it should give you a hint.

Don't iterate over the substrings with for i in xrange(...), if you don't use the index for anything than accessing the string. Instead do

for substring in get_substrings(given_string):
    if substring[0].lower() in "aeiou":
        # ...

Note that I also converted the string to lowercase before the comparison, in case the input string is not all uppercase.

You might also move that counting loop to its own method and return a tuple with the two counts, so that you can write

vowels, consonants = count_vowels_and_consonants(get_sub_strings(given_string))

elif vowels == consonants:
    print("draw")

Note that the specification says to print Draw, not draw, so even if the hackerrank test is lenient one might be pedantic about the fulfillment of the specification.

\$\endgroup\$
  • \$\begingroup\$ Could you by chance review my new code I left as an answer, I have made several new solutions to the challenge but this one is the quickest thus far. Consequently it's also the solution that took some paperwork to figure out! Also, I have a solution utilizing the yield function; I appreciate the advice to use that I learned a lot. \$\endgroup\$ – Anonymous3.1415 Jan 14 '18 at 0:53
  • \$\begingroup\$ Please do not post updates as answers. You can add information to the question by editing it, or if it is a completely new version of the code please open a new question. Add a link to the original one (this) in the new question, and to the new one in an edit of this question. I will then happily review the new version, if you just comment here again afterwards. \$\endgroup\$ – Raimund Krämer Jan 15 '18 at 8:27

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.