Skip to content

Conversation

@hroc135
Copy link
Owner

@hroc135 hroc135 commented Apr 30, 2025

- https://github.com/saagchicken/coding_practice/pull/22/files#diff-641171eaccfc6c3f147cc8650b9302e2abf6fcaf35f2c3c35c8c6e0a4a83d31eR73
- 面白い解法
- なるほど、非同期処理で行番号を生成していくのか
- 道案内人がいて、その人の指示に従ってせっせと文字を追加していく
Copy link

Choose a reason for hiding this comment

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

Go で Python の yield 相当のことをするとメモリーリークするという理解で正しいですか?

そうだとすると何か工夫をする必要がありますね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

あ、channel を Close していないということですね。gemini に聞いたところ、コンテキストを使って関数終了時に goroutine も止めてチャンネルも閉じる方法と、goroutine で必要な回数分 row を生成したときにチャネルを閉じる方法があって、前者の方が複雑な goroutine のライフサイクルの管理に向いているとのことでした。

Copy link
Owner Author

Choose a reason for hiding this comment

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

たいそうなことになってしまったという印象です。

func convert(s string, numRows int) string {
	if numRows == 1 || len(s) <= numRows {
		return s
	}

    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()
	rowChan := make(chan int)
	go func() {
        defer close(rowChan)
		row := 0
		for {
			for row < numRows-1 {
                select {
                case <-ctx.Done():
                    return
                case rowChan <- row:
                    row++
                }
			}
			for row > 0 {
                select {
                case <-ctx.Done():
                    return
                case rowChan <- row:
                    row--
                }
			}
		}
	}()

	rows := make([][]byte, numRows)
	for i := range len(s) {
		r := <-rowChan
		rows[r] = append(rows[r], s[i])
	}
	var sb strings.Builder
	for _, row := range rows {
		for _, b := range row {
			sb.WriteByte(b)
		}
	}
	return sb.String()
}

Copy link

Choose a reason for hiding this comment

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

あ、はい。私も追加で思いつくことが、コンテキストを使うよりは別のチャンネルを作ったほうがシンプルに見えることと、rowChan にサイズ制限をつけることくらいです。select が散るのが嫌ですね。関数化しますか。

rowChan := make(chan int, 100)
done := make(chan struct{})
defer close(done)

Copy link

Choose a reason for hiding this comment

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

DeepSeek 製。

func convert(s string, numRows int) string {
    if numRows == 1 || len(s) <= numRows {
        return s
    }

    rowChan := make(chan int)
    done := make(chan struct{})

    go func() {
        generateRows(done, rowChan, numRows)
    }()

    rows := make([][]byte, numRows)
    for i := 0; i < len(s); i++ {
        r := <-rowChan
        rows[r] = append(rows[r], s[i])
    }
    close(done)

    var sb strings.Builder
    for _, row := range rows {
        sb.Write(row)
    }
    return sb.String()
}

func generateRows(done <-chan struct{}, ch chan<- int, numRows int) {
    defer close(ch)
    row := 0
    for {
        for nextRow := row + 1; row < numRows-1; row = nextRow {
            if !sendRow(done, ch, row) { return }
            nextRow = row + 1
        }
        for nextRow := row - 1; row > 0; row = nextRow {
            if !sendRow(done, ch, row) { return }
            nextRow = row - 1
        }
    }
}

func sendRow(done <-chan struct{}, ch chan<- int, row int) bool {
    select {
    case <-done:
        return false
    case ch <- row:
        return true
    }
}

- とにかく grid の大きさに依存することはわかるがしっかりとした見積もりはよくわからない
- 時間計算量: ??
- 外側のループは len(s) だけ回り、append で numRows の定数倍時間かかる
- ただし、append で reallocate が起きる場合は grid の列数に依存するので空間計算量同様よくわからない
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.

列数は
len(s) / (2*numrows-2) * (numrows-1)
で決まるというのを誰かのPRで見ました。
ということは len(s)/2 ですね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

つまり、step1の場合、
空間計算量: O(numrows * len(s))
時間計算量: O(len(s) * max(len(s), numrows))
あ、「大きい方」ってこういうことですか?

rows := make([][]byte, numRows)
row := 0
isGoingDown := true
for i := range s {
Copy link

Choose a reason for hiding this comment

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

ここを for i, r := range s {} のように扱わなかったのは s[i] で byte 型としたかったからですかね?

今回の問題設定の場合、文字は英語(大文字・小文字)と , および . で ASCII の範疇なので byte 型で扱っても問題なさそうですが、制約がマルチバイト文字に拡張された場合だとうまく動かなさそうかなと思いました。

rows := make([][]rune, numRows) と宣言して、for _, r := range s {} のように扱えば r が rune 型なのでマルチバイト文字でも適切に扱えそうです。

Comment on lines +226 to +237
switch isGoingDown {
case true:
row++
if row == numRows-1 {
isGoingDown = false
}
case false:
row--
if row == 0 {
isGoingDown = true
}
}
Copy link

Choose a reason for hiding this comment

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

isGoingDown: bool を使わずに、1 または -1 をもつ direction: int のようなものを使う書き方だと、上記の switch 文の処理を共通化できます。row += direction とし、折り返し地点が来たら反転させる(direction *= -1 とする)イメージです。

ただ direction は実際には 1 と -1 しか扱わないものの int 型からは2値であることはわからないので、その点が不明瞭なのはデメリットかもしれません。適宜コメントなどで補う必要がありそうです。

Copy link
Owner Author

@hroc135 hroc135 May 1, 2025

Choose a reason for hiding this comment

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

マルチバイト文字への対応も含めて書いてみました。かなりすっきりしたコードになりました。

func convert(s string, numRows int) string {
	if numRows == 1 || len(s) <= numRows {
		return s
	}
	rows := make([][]rune, numRows)
	row := 0
	direction := 1 // direction only contains either 1 or -1
	for _, c := range s {
		rows[row] = append(rows[row], c)
                row += direction
                if row == 0 || row == numRows - 1 {
                        direction *= -1
                }
	}
	var sb strings.Builder
	for r := range rows {
                for _, c := range rows[r] {
                        sb.WriteRune(c)
                }
	}
	return sb.String()
}

Copy link

Choose a reason for hiding this comment

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

僕の方で想定していたコードとほぼ同じです。また個人的には、処理のブロックごとに適宜改行を入れるのが区切りがわかりやすくて好きです。

今回のコードだと、大きく「エッジケース処理」、「Zigzag に変換する処理」、「文字列を構築する処理」の3つくらいに区切れそうなので、rows := make([][]rune, numRows)var sb strings.Builder の上辺りに改行を入れるかなと思いました。(あくまで自分だったら、という話なので参考までに。)

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.

4 participants