6
\$\begingroup\$

Problem Statement

Suppose you have a string \$S\$ which has length \$N\$ and is indexed from \$0\$ to \$N−1\$. String \$R\$ is the reverse of the string \$S\$. The string \$S\$ is funny if the condition \$|S_i−S_{i−1}|=|R_i−R_{i−1}|\$ is true for every \$i\$ from \$1\$ to \$N−1\$.

(Note: Given a string \$str\$, \$str_i\$ denotes the ascii value of the \$i\$th character (0-indexed) of \$str\$. \$|x|\$ denotes the absolute value of an integer \$x\$)

Example

is_funny("acxz") returns True

is_funny("bcxz") returns False

Solution

def is_palindrome(ary):
    return ary == ary[::-1]

def create_difference_list(s):
    res = []
    for i in range(1, len(s)):
        res.append(abs(ord(s[i]) - ord(s[i-1])))
    return res

def is_happystr(s):
    return is_palindrome(create_difference_list(s))

Notes

  • Have used slow version of palindrome check assuming string is not going to be too long. Hope it doesn't affect performance that much.
  • Approached problem via functional style, could have done all the computation in the same loop but is it a good practice?
\$\endgroup\$
5
\$\begingroup\$

Answering your specific questions:

Have used slow version of palindrome check assuming string is not going to be too long. Hope it doesn't affect performance that much.

You made a very good choice. This solution is simpler and more readable, you may switch to a new faster implementation later iff performance becomes a problem and you are sure is_palindrome is the problem.

Approached problem via functional style, could have done all the computation in the same loop but is it a good practice?

To be precise on terminology, you used an imperative style with subroutines. I think that the use of subroutines ("functions") like you do greatly improves readability, so well done.


You may re-factor into functions further by writing a function to return pairs from a list and calling it from create_difference_list

\$\endgroup\$
  • \$\begingroup\$ Hmm, but if we just look at the is_happystr then its seems functional style isn't it? provided we don't mutate the original data in between. Also, I find that while problem solving it helps a lot to divide the problem in small pieces like this. Instead of writing whole logic in one place. \$\endgroup\$ – CodeYogi Sep 17 '15 at 17:12
  • \$\begingroup\$ So, overall its a good programming style as I infer from your answer. But there is a problem I asked where I was needed to calculate the sum of even fibonacci terms, there I was strongly criticized for using a separate fib function which returned the nth fib term. I was instead asked to have an iterative solution with sum calculation part inline. \$\endgroup\$ – CodeYogi Sep 17 '15 at 17:15
  • \$\begingroup\$ I can give you that link if you want. \$\endgroup\$ – CodeYogi Sep 17 '15 at 17:16
  • \$\begingroup\$ Here it is Even fibonacci numbers, even you have gave your version of solution there. But I was strongly criticized by others. \$\endgroup\$ – CodeYogi Sep 17 '15 at 17:22
  • \$\begingroup\$ @CodeYogi criticized because in that case the function was not needed and just added complexity \$\endgroup\$ – Caridorc Sep 17 '15 at 17:46
3
\$\begingroup\$

You could create the result of create_difference in one line with a list comprehension, that way you can return it directly. A list comprehension collapses a for loop into one line, here's how it'd look:

def create_difference_list(s):
    return [abs(ord(s[i]) - ord(s[i-1])) for i in range(1, len(s))]
\$\endgroup\$
  • \$\begingroup\$ I don't go for one liners. I would instead create a summary variable like abs_diff which can let the reader know what's going on. wdyt? \$\endgroup\$ – CodeYogi Sep 18 '15 at 4:40
  • \$\begingroup\$ @CodeYogi I think if you're concerned with informing the user then a docstring is more useful. abs_diff isn't entirely clear by itself, while a sentence in a docstring could tell the user about the function in a clear way. \$\endgroup\$ – SuperBiasedMan Sep 18 '15 at 9:06
1
\$\begingroup\$

create_difference_list sounds weird to me; difference_list sounds nicer. We don't have create_reversed or create_enumerate, after all.

Anything of the form

foo = []
for elem in elems:
    foo.append(func(elem))

is more neatly expressible as

foo = [func(elem) for elem in elems)]

given it's of reasonable length, so in this case I would use

def difference_list(s):
    return [abs(ord(s[i]) - ord(s[i-1])) for i in range(1, len(s))]

as SuperBiasedMan suggests.

However, there is a standard recipe for pairwise iteration:

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = tee(iterable)
    next(b, None)
    return zip(a, b)

which you should use, since it's standard.

\$\endgroup\$
  • \$\begingroup\$ Agree but not completely, if every line of your code requires your reader to lookup documentation then it defies the concept of readability all together, trickery should be limited to Code Golf. \$\endgroup\$ – CodeYogi Sep 20 '15 at 15:56

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.