Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

タイムゾーンを跨いだ場合に新しいタイムゾーンで計算されない #31

Merged
merged 2 commits into from Oct 4, 2015

Conversation

akuraru
Copy link
Collaborator

@akuraru akuraru commented Oct 4, 2015

日本語で失礼します。いつも有り難く使わせて頂いております!

AZ_currentCalendar メソッドを呼び出した際に、以前に使用済みであればキャッシュしておいたものを使い回すように作られていますよね。

ここで、タイムゾーンを跨ぐ場合、例えば dateAtStartOfDayメソッドであれば、変更後のタイムゾーンの 00:00:00の NSDateを返すことを期待する事になると思うのですが、キャッシュされた NSCalendarの保持する timezoneプロパティの情報を利用する為か、生成時のタイムゾーンのままで計算されてしまうようです。

なので、AZ_currentCalendarを呼び出す際には常にその時の [NSTimezone systemTimezone]等で更新しておくようにした方が期待した動作になると考えたのですが如何でしょうか。

ご検討をお願いします!

@azu azu added the enhancement label Oct 1, 2015
@azu
Copy link
Owner

azu commented Oct 1, 2015

一度AZ_currentCalendarを叩いた後に、ユーザーがタイムゾーンの異なる場所へ移動 or タイムゾーンを明示的に変更した際に、最初にキャッシュされたタイムゾーンになってしまうということですよね。

calendarをキャッシュしてるのはNSCalendarの生成が重たかったためでしたが、[NSTimezone systemTimezone]とかはどうなんだろ?
ドキュメントを見る感じだと[NSTimezone systemTimezone]は自動でキャッシュされてて、resetSystemTimeZoneでリセットみたいになってそうですね。

なので、AZ_currentCalendar内で毎回代入しても影響は小さいんじゃないかなと思いますがどうでしょう? > @akuraru

@akuraru
Copy link
Collaborator

akuraru commented Oct 1, 2015

@azu @ochimasu 性能テストします。影響が十分に小さい場合、採用します。

@akuraru
Copy link
Collaborator

akuraru commented Oct 4, 2015

多少の性能劣化はあったもの、大きく劣化しないということがわかった。
@azu 相談
AZ_currentCalendarが呼び出されるたびにresetSystemTimeZoneをすると非常に性能に影響する(+0.15s → +4.02s と 25.8倍劣化する)。resetSystemTimeZoneのタイミングについてはライブラリのユーザーにまかせることにしたいのだがそれでよいだろうか?このライブラリは基本的に「便利メソッドを提供する」ものなので、勝手にresetSystemTimeZoneを使って状態を変えるのは良くないと思っている。ochimasu さんの意図とは少しそぐわない形になってしまうが、仕方ないということで。

@azu
Copy link
Owner

azu commented Oct 4, 2015

resetSystemTimeZoneのタイミングについてはライブラリのユーザーにまかせることにしたいのだがそれでよいだろうか?

👍 Agreeです。
これをライブラリ側でやるとコストが高すぎるので、autoupdatingCurrentCalendarみたいに標準のものがあるならいいのですが、そうでない場合はユースケース的にもやり過ぎになると思います。

@ochimasu
Copy link
Author

ochimasu commented Oct 4, 2015

[NSTimezone systemTimezone] なのですが、とりあえず自分のiOS9端末で試した所、 設定 -> 日付と時刻 -> 時間帯 でタイムゾーンを直接変更した場合には、その後で resetSystemTimeZone を呼ばなくても、次に systemTimezone を呼び出した場合には新しいタイムゾーンが呼び出されている事を確認しています。
なので、自分のユースケースとしても systemTimezone を呼び出すだけで十分だと予想しています。

むしろ、これで更新されているのなら、明示的にresetSystemTimeZoneを呼ばなければならない状況というのがよくわからないわけですが・・・

@akuraru
Copy link
Collaborator

akuraru commented Oct 4, 2015

@azu 一応確認お願いします。(一行しか変わってないけど

akuraru added a commit that referenced this pull request Oct 4, 2015
タイムゾーンを跨いだ場合に新しいタイムゾーンで計算されない
@akuraru akuraru merged commit 0a95f4a into azu:master Oct 4, 2015
@akuraru
Copy link
Collaborator

akuraru commented Oct 4, 2015

@ochimasu 1.5.2 をリリースしました。

@ochimasu
Copy link
Author

ochimasu commented Oct 5, 2015

@akuraru @azu ありがとうございます!今後も使わせてもらいます!

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

Successfully merging this pull request may close these issues.

None yet

3 participants