Skip to content

Create 6. Zigzag Conversion.md#60

Open
tokuhirat wants to merge 1 commit intomainfrom
6.-Zigzag-Conversion
Open

Create 6. Zigzag Conversion.md#60
tokuhirat wants to merge 1 commit intomainfrom
6.-Zigzag-Conversion

Conversation

@tokuhirat
Copy link
Owner

This Problem
6. Zigzag Conversion
言語: Python

num_letters_in_cycle = 2 * numRows - 2
for i, letter in enumerate(s):
position_in_cycle = i % num_letters_in_cycle
if position_in_cycle < numRows:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

row_index = min(position_in_cycle, num_letters_in_cycle - position_in_cycle) とも書けると思うのですが、パズルになってしまって読みにくくなるので、避けたほうが無難だと思います。

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

私も同感です。

else:
row_index = num_letters_in_cycle - position_in_cycle
row_to_letters[row_index].append(letter)
return "".join("".join(row) for row in row_to_letters)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

join() が 2 重になっているのがやや読みづらく感じました。 2 行に分けると読みやすくなるかもしれません。

rows = ["".join(row) for row in row_to_letters]
return "".join(rows)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rows = ("".join(row) for row in row_to_letters)

とすると、Generator で副作用がある場合は等価ではないですね。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

差異があるケースは以下のようにあると思いますが、想定されているケースと合致していますか?

counter = 0


def f(row):
    global counter
    counter += 1
    return "".join(row) + str(counter)


row_to_letters = [["A"], ["B"], ["C"]]


counter = 0
rows = [f(row) for row in row_to_letters]
counter = 100
print("".join(rows))  # A1B2C3

counter = 0
rows = (f(row) for row in row_to_letters)
counter = 100
print("".join(rows))  # A101B102C103

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そうですね。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。意外と気にすることが多いですね。
他には itertools.chain も使えるかなと思いました。
https://docs.python.org/3/library/itertools.html#itertools.chain

import itertools
print("".join(itertools.chain.from_iterable(row_to_letters)))

- https://github.com/olsen-blue/Arai60/pull/61/files#r2042314581
- 以下の二つは同じ
`"".join("".join(row_chars) for row_chars in rows)`
`"".join(c for row_chars in rows for c in row_chars)`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

内包表現を 2 重にすると読みづらく感じます。原則避けたほうが良いと思います。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、これ結果は同じですが、上は文字列を複数回 join していますが、下は Generator を利用して join 一回です。

return s

row_to_letters = [[] for _ in range(numRows)]
num_letters_in_cycle = 2 * numRows - 2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この行は、2倍する意味や-2する意味のコメントがあると親切だと思いました。


row_to_letters = [[] for _ in range(numRows)]
row_index = 0
direction = 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

私は bool の方が読みやすく感じますが好みの範囲だと思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants