Proposal: (Period[T]) Periodic and similar methods returns iter.Seq[Time[T]] instead of channel

[nobishino]

golang/go#61897 がacceptedになりました。

これに伴い、コレクションを返すAPIではiter.Seqを返すことがおそらく一般的になっていくと思います。
例えばGo標準ライブラリのproposal golang/go#53987 (comment) は、最終的にiter.Seqを返すAPIとしてacceptされました。

同様に、synchroでコレクションを返すAPIをiter.Seqに対応させるプランはあるでしょうか？（もしあれば、実装することに興味があります）

具体的には以下のようなものです。
細かいところが十分考えられていないのですが、少し見ていただいた印象としてもしポジティブであればより具体的に考えたいと思っています。

Proposal

synchroではコレクションを返すメソッドとしてPeriod[T]などがunexportedな型periodical[T]を返しており、これの実体は<-chan Time[T]になっていると思います。これの代わりに、iter.Seqを返すようにするというProposalです。

func (Period[T]) Periodic() iter.Seq[Time[T]]

Edit: 最初aliasを使う形で書いていたのですが、言語仕様上できないことに気づき、修正しました。

メリット

• proposal: x/exp/xiter: new package with iterator adapters golang/go#61898 のようなiter.Seqを前提としたAPIと組み合わせられるようになること
• 将来的に標準的になりそうなAPIに合わせられること

実は具体的なユースケースが手元にあるわけではなく、これを提供することで大きいメリットがあるかどうかはわかっていません。

互換性などについてのノート

nobishino@1fd8814 でテストを書きましたが、少なくとも一部で後方互換性がありません。

• もともとはperiodical型がchannelなので、channel型の変数に代入できるという性質(テストのProperty 1)があった。periodicalの型定義をiter.Seqに置き換えると、この代入はできなくなる。
• もともとperiodical型がchannelなので、for ~ range文で直接使うこともできた(テストのProperty 2)。この性質はrange over funcが実装された後のGoバージョンでは維持される。
• Sliceメソッドを削除する必要が生じる。
  • ※Sliceメソッドを定義できるようにtype periodical[T TimeZone] iter.Seq[Time[T]]を戻り値とした場合、戻り値の値をiter.Seq[Time[T]]型の変数(関数の引数も含む)にそのままでは代入できなくなる。（型変換をすればできると思われる）

[Code-Hex]

Hello @nobishino !! Thank you for describing the proposal. I have indeed considered iterators in the past.

Regarding the proposal you have presented, honestly, I don't feel like the benefits outweigh the disruption to backward compatibility.

func (p Period[T]) Periodic(next func(Time[T]) Time[T]) periodical[T]

If there are benefits to implementing the iterator interface, perhaps we could provide an Iter() method similar to the Slice() method, so that we can enjoy the benefits you are proposing.

What do you think?

[nobishino]

Thank you! I would add some more detail and options for my proposal.

First of all, my proposal is motivated not by specific use-case, but by my personal technical curiosity.
So feel free to decline for not having specific use-case 😸

honestly, I don't feel like the benefits outweigh the disruption to backward compatibility.

Thanks for feedback! Based on that, I came up with several options which keeps backward compatibility as follows.
Option 2, 2-2, and 3 are illustrated in my draft branch: https://github.com/nobishino/synchro/commits/wip-period-sequence-api

Option 1: Add no new API for iter.Seq

This is obvious and completely reasonable option 😃

pros:

• No API Addition.

cons:

• Not enjoying benefits which iter ecosystem would provide.

Option 2: Add Seq() method to periodical[T]

This corresponds to your Iter() method idea. (I wrote this as Seq accidentally, and don't have strong preference for either name.)

Sample implementation for this option is given here:
nobishino@f3e5495

Since iter package is not yet provided as standard library, I write the sample using my fake package github.com/nobishino/gocoro/iter.

pros:

• Simple to implement
• Add least new API (only 1 method to already existing type)

cons:

• When break statement is used, which is translated to false return value from yield function, the goroutine associated with periodical instance is not terminated. (illustrated in the next commit's test: nobishino@608b669)
  • And, I think users don't expect iter.Seq[Time] cause such resource leakage for break statement.

Option 2-2: Same as Option 2 but address goroutine termination

This is variant of Option 2, but addresses the goroutine termination issue.

If we keep backward compatibility, we would need somewhat awkward implementation like nobishino@608b669

pros:

• Add least new API (only 1 method to already existing type).
• No goroutine leakage.

cons:

• Implementation would be complex and awkward.

Option 3: Add new methods to Period[T] type which returns iter.Seq

Option 3 adds several methods to Period[T](not periodical[T]) which have same functionality to PeriodicXXX family but returns iter.Seq instead. This option needs following addition to Period[T]'s API:

• func (p Period[T]) Sequence(next func(Time[T]) Time[T) iter.Seq[Time[T]]
• func (p Period[T]) SequenceDuration(d time.Duration) iter.Seq[Time[T]]
• func (p Period[T]) SequenceDate(years int, months int, days int) iter.Seq[Time[T]]
• func (p Period[T]) SequenceAdvance(u1 Unit, u2 ...Unit) iter.Seq[Time[T]]
• func (p Period[T]) SequenceISODuration(duration string) iter.Seq[Time[T]]

Illustrated in nobishino@336c474

pros:

• Simple to implement.
• No goroutine leakage.

cons:

• 5 new public API needs to be added.

Summary

I have explained 4 options which keep backward compatibility. Comparing these, there seems several objectives we want to meet, but I couldn't find the way to meet all of these.

1. Conform to & enjoy benefits of iter ecosystem
2. Have least addition of public APIs
3. Have simple & readable implementation
4. Make sure no resource leakage is caused by natural usage

My personal preference is Option 3, which meets 1, 3, 4 but giving up 2.
If we think the benefit of 1 is not too huge, I would think Option 1 (not adding new feature at least for now) is good decision.

[Code-Hex]

Thank you for the wonderful proposal of Pros and Cons, @nobishino. It has become clear to me how we can approach each of them.

The policy I support is as follows:

• I don't want to increase the number of APIs that are currently unlikely to be used.
  • The reason is that having similar APIs can lead to confusion among users about which one to use.
• I don't want to return types that depend on third-party packages.
  • This is because we would lose control if we depended solely on this package. There is a possibility of performing Breaking Changes when maintenance stops or when bugs occur that require fixes dependent on external factors. I want to avoid this as much as possible.

As for the direction, I think Option 2, 2-2 is better.

Although it was mentioned that it might become complicated, I believe that by implementing it in this way, where all processing for iteration is enclosed in the Seq() method, it won't become complex. What do you think?

func (p periodical[T]) Seq() iter.Seq[Time[T]] {
	return func(yield func(Time[T]) bool) {
		for current := range p {
			if !yield(current) {
				break
			}
		}
		go func() {
			for current := range p {
				// Prevents leakage of the sending goroutine by reading
				// all channels sent to periodical[T].
			}
		}()
	}
}

[nobishino]

@Code-Hex Oh, that is very nice! I didn't come up with that receiver-side solution.

Now I am convinced that Option 2-2 (with your implementation) is good idea.

[nobishino]

I would submit a PR when iter.Seq is released(or about to be released).

Here is WIP branch: nobishino@ce1f98f
I will continue from this WIP when iter.Seq is ready to use.
(By the way, that PR would need to update go.mod to require Go1.23.0)

[Code-Hex]

@nobishino I'm sorry for late response.
I updated go version in #39