HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Hackerrank Funny String python solution

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
funnypythonhackerranksolutionstring

Problem

"Funny String" problem from Hackerrank


Suppose you have a String, \$S\$ of length \$N\$ indexed from \$0\$ to \$N-1\$. You also have some String, \$R\$, that is the reverse of \$S\$, where \$S\$ is funny if the condition is \$\left|S[i] - S[i-1]\right| = \left|R[i] - R[i-1]\right|\$ is true for all \$i \in [1, N-1]\$.

I code mainly in Java. I'm very new to Python, so I would appreciate feedback on how I can make this code more Pythonic, cleaner and more efficient in general.

import math

def isFunny(word):
    i = 0
    length = len(word)
    arr = []
    revarr = []
    for i in range(0,math.ceil(length/2)):
        sdiff = abs(ord(word[i])-ord(word[i+1]))
        rdiff = abs(ord(word[length-i-1])-ord(word[length-i-2]))
        if sdiff == rdiff:
               continue
        else:
               return False
    return True

if __name__ == "__main__":
    n = int(input())
    for _ in range(n):
        word = input();
        if isFunny(word):
            print("Funny")
        else:
            print("Not Funny")

Solution

Code organisation and other things well done

You've extracted the algorithm in a function on its own. Also, you've written you code behind the "if name == main" guard. Congratulations on this, these are two good things beginners usually don't do properly.
This makes your code clearer, easier to maintain and easier to test.

Also, I'd like to take this chance to congratulate you for using _ as a name for a throw-away value which is quite a nice habit.

Finally, you did notice that there was not point in looping over the whole string once you've reached the middle.

Getting rid of useless things


It seems that perfection is attained not when there is nothing more to add, but when there is nothing more to remove. - Antoine de Saint Exupéry

I can see in your code artifacts of old versions of your code : arr and revarr are not used at all. Also you don't need to define i before the loop.

continue is nice keyword, available in most programming languages. However, from my experience, you don't need it that often. In your case, you might as will just write :

if sdiff != rdiff:
           return False


Finally, 0 is not required as a first argument to range.

Style

Python has a style guide called PEP 8. It is definitly worth a read. You'll find various tools online to check your code's compliancy to PEP 8. From what I can see, the main "issue" is that the function name is CamelCased and not snake_cased.

Documentation

If you wanted to do things properly, it might be interesting to add some documentation to your function.

More details (and personal preferences)

We have a nice ternary operator in Python. Using it, you could write :

print("Funny" if is_funny(word) else "Not Funny")


Also, your is_funny function looks like the typical situation where you could use the all/any Python builtin. However, because of the length of the expressions involved, it becomes a bit of a matter of personal preference. I'd write this (but you don't have to like it) :

def is_funny(word):
    """Check if a word is funny."""
    length = len(word)
    return all(
        abs(ord(word[i])-ord(word[i+1])) == abs(ord(word[length-i-1])-ord(word[length-i-2]))
        for i in range(math.ceil(length/2))
    )


Another thing that could be changed but might not make the code clearer to every one is the way you get the n-th element of the reversed string. You can use word[length-i-1] like you did but you can also take the most out of Python's index handling : negative indices are taken from the end of the container. So that you can write word[-i-1] instead (and the same principle applies to word[length-i-2]).

Code Snippets

if sdiff != rdiff:
           return False
print("Funny" if is_funny(word) else "Not Funny")
def is_funny(word):
    """Check if a word is funny."""
    length = len(word)
    return all(
        abs(ord(word[i])-ord(word[i+1])) == abs(ord(word[length-i-1])-ord(word[length-i-2]))
        for i in range(math.ceil(length/2))
    )

Context

StackExchange Code Review Q#130049, answer score: 5

Revisions (0)

No revisions yet.