10
\$\begingroup\$

I am just trying to solve one of the puzzle which is explained below:

The sentence if man was meant to stay on the ground god would have given us roots after removing spaces is 54 characters long, so it is written in the form of a grid with 7 rows and 8 columns.

The encoded message is obtained by displaying the characters in a column, inserting a space, and then displaying the next column and inserting a space, and so on. For example, the encoded message for the above rectangle is:

imtgdvs fearwer mayoogo anouuio ntnnlvt wttddes aohghn sseoau

For more conditions please refer here:

from math import ceil, sqrt
from string import whitespace

def encrypt(string):
    enc_string = string.translate(None,whitespace)
    length = len(enc_string)

    #calculate the no of colums for the grid
    columns = int(ceil(sqrt(length)))

    #form a list of strings each of length equal to columns
    split_string = [enc_string[i:i+columns] for i in range(0, len(enc_string), columns)]

    #encrypted list of strings
    new_string = []
    for string in split_string:

        for i in range(len(string)):
            try:
                #replace the element in the list with the new string
                new_string[i] = new_string[i].replace(new_string[i], new_string[i] + string[i])
            except IndexError:
                #exception indicates that the string needs be added with new Index
                new_string.append(string[i])

    return " ".join(new_string)

if __name__ == "__main__":
    s = raw_input().strip()
    result = encrypt(s)
    print(result)

Please help me if multiple for loops can be avoided and any other improvements.

\$\endgroup\$
  • 1
    \$\begingroup\$ Can you add, in the description, what are the rules to compute the lengths of the rectangle and how "missing" values at the end should be handled? \$\endgroup\$ – 409_Conflict Jan 5 '18 at 11:06
  • \$\begingroup\$ @MathiasEttinger: have provided the link to refer for more information, didn't get wt "missing" values you are talking about ? \$\endgroup\$ – Here_2_learn Jan 5 '18 at 15:03
14
\$\begingroup\$

That seems like a very long method to splice and put together the string every even set of characters. You are essentially trying to build a square matrix using the characters from the given string. Python has a very nice slice-and-stride feature which can perform the same task in a single inline operation.

I took the liberty of rewriting your encrypt function:

from math import ceil
from string import whitespace

def square_encrypt(str_input): # avoiding the name `string` as python has a module `string`
    str_input = str_input.translate(None, whitespace) # making a copy and overwriting input parameter
    square_size = int(ceil(len(str_input)**0.5)) # or int(ceil(sqrt(len(str_input))))
    encrypted_list = [str_input[i::square_size] for i in xrange(square_size)]
    return " ".join(encrypted_list)
\$\endgroup\$
  • \$\begingroup\$ You're already pulling in math.ceil, so why raise to the power 0.5 instead of use math.sqrt? For efficiency? \$\endgroup\$ – RonJohn Jan 5 '18 at 16:00
  • \$\begingroup\$ @RonJohn I copied over the code from my terminal; where I had not imported sqrt. The performance is very slightly different between the two. Though, using sqrt is more readable. :) \$\endgroup\$ – hjpotter92 Jan 5 '18 at 16:25
  • \$\begingroup\$ can you mention the reason for using "xrange" over "range"? I refered here : stackoverflow.com/questions/94935/…. I didn't get the meaning "xrange is a sequence object that evaluates lazily". If possible could you explain this. \$\endgroup\$ – Here_2_learn Jan 6 '18 at 15:14
  • 1
    \$\begingroup\$ @Here_2_learn The official documentation explains it better devdocs.io/python~2.7/library/functions#xrange \$\endgroup\$ – hjpotter92 Jan 6 '18 at 15:20
13
\$\begingroup\$

One of your lines of code is horribly cryptic. I think it's so bad it's worthy of it's own answer.

new_string[i] = new_string[i].replace(new_string[i], new_string[i] + string[i])

This is a longwinded way of writing:

new_string[i] += string[i]

This is something you should have noticed, and should have gone to first.


If you change your for loop to use enumerate too, then the above line becomes much cleaner.

for i, char in enumerate(string):
    try:
        new_string[i] += char
    except IndexError:
        new_string.append(char)

You can also get rid of the try, if you fill your new_string with empty strings, on creation.

new_string = ['']*columns
for string in split_string:
    for i, char in enumerate(string):
        new_string[i] += char

However hjpotter92's answer is still what you should do.

\$\endgroup\$
  • \$\begingroup\$ Really cool improvements, and Thanks for being frank "horribly cryptic" \$\endgroup\$ – Here_2_learn Jan 6 '18 at 15:08

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.