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

Fix UNIX time parsing (value passed by JSON.NET is long) #67

Merged
2 commits merged into from Jul 4, 2014

Conversation

Projects
None yet
2 participants
@a4lg
Contributor

a4lg commented Jul 3, 2014

JSON.NET から UNIX タイムとして渡される値が double ではなく long であるため、reader.Value の unboxing に失敗します (ここで扱われる UNIX タイムは今のところ API レート取得 API [application/rate_limit_status] においてのみ確認しています)。そのため、追加のキャストによってこの問題を解決します。
この問題ですが……どうも CoreTweet 側というより JSON.NET 側の極端なわかりづらさによって生まれたバグに見えます。個人的には、最悪 JSON.NET から別の JSON ライブラリに移行することを考えた方が良いのではないか、と考えます (.NET 3.5 から標準の JSON シリアライザがあるので、それで何とかならないか試行錯誤しています)。

a4lg added some commits Jul 3, 2014

DateTime optimization
There's two kinds of optimization in this commit.

*	Common UNIX epoch object to make efficient and easy to read.
*	Use `Ticks' rather than `Seconds' in floating point values.

ghost pushed a commit that referenced this pull request Jul 4, 2014

Merge pull request #67 from a4lg/unix-time-fix
Fix UNIX time parsing (value passed by JSON.NET is long)

@ghost ghost merged commit f2ef445 into CoreTweet:master Jul 4, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@azyobuzin

This comment has been minimized.

Show comment
Hide comment
@azyobuzin

azyobuzin Jul 4, 2014

Member

昔はその .NET 標準の JsonReaderWriterFactory から XElement を作って dynamic でラップしてました(DynamicJson)。このやり方はコード量が増える……。

Member

azyobuzin commented Jul 4, 2014

昔はその .NET 標準の JsonReaderWriterFactory から XElement を作って dynamic でラップしてました(DynamicJson)。このやり方はコード量が増える……。

@a4lg a4lg deleted the a4lg:unix-time-fix branch Jul 4, 2014

@a4lg

This comment has been minimized.

Show comment
Hide comment
@a4lg

a4lg Jul 4, 2014

Contributor

なるほど了解しました。JSON.NET 側の仕様のちゃんとした把握でバグ減らす方向に持っていくのが良さそうですね。
それと pull request 間違えて別の最適化含むコミット (commit 3267c16) が含まれていることをお伝えしておきます。UNIX タイムから日時への変換で使われる double 引数を long にした (AddSeconds 等で使われるインタフェース用の単位を持つ実数でなく、100ns 単位の Tick という内部的単位を直接操作した) ことで、最初のコメントで言及している 2 つのキャストのうち 1 つを減らし、また効率をほんの少し上げています。

Contributor

a4lg commented Jul 4, 2014

なるほど了解しました。JSON.NET 側の仕様のちゃんとした把握でバグ減らす方向に持っていくのが良さそうですね。
それと pull request 間違えて別の最適化含むコミット (commit 3267c16) が含まれていることをお伝えしておきます。UNIX タイムから日時への変換で使われる double 引数を long にした (AddSeconds 等で使われるインタフェース用の単位を持つ実数でなく、100ns 単位の Tick という内部的単位を直接操作した) ことで、最初のコメントで言及している 2 つのキャストのうち 1 つを減らし、また効率をほんの少し上げています。

@azyobuzin

This comment has been minimized.

Show comment
Hide comment
@azyobuzin

azyobuzin Jul 4, 2014

Member

👍

効率以上にぱっと見でわかりにくいな、と思ってしまいました。ええ、 C# ゆるふわ勢です。

Member

azyobuzin commented Jul 4, 2014

👍

効率以上にぱっと見でわかりにくいな、と思ってしまいました。ええ、 C# ゆるふわ勢です。

This issue was closed.

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