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

1文字の比較で wcsncmp 関数を使うのは止める #853

Merged
merged 1 commit into from
Apr 15, 2019
Merged

1文字の比較で wcsncmp 関数を使うのは止める #853

merged 1 commit into from
Apr 15, 2019

Conversation

beru
Copy link
Contributor

@beru beru commented Apr 14, 2019

1文字の比較でわざわざ関数呼び出しを行うと余分に処理時間が掛かってしまうので wcsncmp 関数を使わずに直接比較するように変更しました。

bracket に surrogate pair を使う事は無いと思います。

@beru beru added the 🚅 speed up 🚀 高速化 label Apr 14, 2019
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

LGTMです。

変更の趣旨と内容は問題ないと思います。
なんで比較にCRT使っとんのや! と前からナゾでした。

「C言語の論理で書いたほうが分かりやすくて高速だ」と信じてる人が、かつては多数派だったんじゃないか?と予想していますが、これは「必ずしも真ではない」と思っています。

言語 文字列型 比較関数 比較演算子
普通の言語 string compare ==など
C言語 ない strcmpなど ない

C++プログラムでは普通、値の比較に演算子を使います。
たくさんある比較関数の細かな違いを覚えて使い分けるより、
数種類しかない演算子を使うほうがより直観的に書けるからです。

「~が~な状態とは?」を突き詰めた実装をしたいときには、
独自の演算子を定義したり、std::char_traitsを派生させたりするもんだと思っています。

1文字の比較でわざわざ関数呼び出しを行うと余分に処理時間が掛かってしまうので wcsncmp 関数を使わずに直接比較するように変更しました。

名前 疑似コード 説明
変更前 wcsncmp( (wchar_t*) szA, &((wchar_t*)szB)[index], 1) == 0 文字列の先頭N文字を比較する関数を 呼出 して比較している。関数呼出を伴うため遅い。
変更後 ((wchar_t*)szA)[0] == ((wchar_t*)szB)[index] 文字列の指定したindexにある「値」を直接比較している。単なる数値比較のため速い。

個人的には *(p->sStr) == cline[ptPos.x] ではなくて cline[ptPos.x] == p->sStr[0] だと思いますが、その辺は趣味だと思うのでどっちでもいいです。(設計書に起こす場合に「行データのN文字目が定数XXXと等しい場合」と自然に書けるからです。

@beru
Copy link
Contributor Author

beru commented Apr 15, 2019

レビューありがとうございます。それでは Merge します。
もし問題が見つかったら別PRで対処します。

等価演算子の左項と右項の順序についてですが、元々 wcsncmp 関数に渡していた順序のままで変更しました、特に何も考えずに。

@beru beru merged commit 846ce28 into sakura-editor:master Apr 15, 2019
@berryzplus
Copy link
Contributor

等価演算子の左項と右項の順序についてですが、元々 wcsncmp 関数に渡していた順序のままで変更しました、特に何も考えずに。

まー、それも含めて approve なんで、なんかあったら「悪りぃ」です。

@m-tmatma m-tmatma added this to the v2.4.0 milestone Apr 28, 2019
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
1文字の比較で wcsncmp 関数を使うのは止める
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants