CNative::Clear() → CNative::shrink_to_empty() に名前変更する

[m-tmatma]

対応の背景

#776 でウィンドウタイトルを取得するユーティリティを実装する際に #776 (comment) のコメントをもらった。

それで #776 (comment) のコメントの通り
データサイズを空にするべく #777 を作成した。

そうした場合、#777 (review) のようにメモリサイズを減らしたい場合に対応できないというコメントをもらった。

対応内容

データサイズを空にするという処理を CNative::Clear() で実装することにして
std::basic_string::shrink_to_fit に近いバッファサイズを縮小する既存の CNative::Clear() を名前変更することにする。

この PR マージ後に #777 を rebase して、 CNative::Clear() でデータサイズを空にする処理を実装する予定。

[berryzplus]

経緯は了解。スジが通ってると思います。
CNative::shrink_to_empty() の「名前が適切か？」に引っかかっていて少し時間がほしいです。

[m-tmatma]

CNative::shrink_to_empty() の「名前が適切か？」に引っかかっていて少し時間がほしいです。

単に名前だけの問題だったら、後で一括置換してもいいと思います。
すぐに思いつかないのだったら、時間をかける必要はないと思います。

[berryzplus]

CNative::shrink_to_empty() の「名前が適切か？」に引っかかっていて少し時間がほしいです。

単に名前だけの問題だったら、後で一括置換してもいいと思います。
すぐに思いつかないのだったら、時間をかける必要はないと思います。

詳細を一言で表すものが関数名だと思います。
名前に違和感を覚えるってことは、詳細に違和感があるってことでもあります。

wstring::shrink_to_fit() のように使いたいケースのために別関数を用意する、まではいいです。
じゃあなんで shrink_to_fit() にしないんでしたっけ？

CMemoryにメモリ縮小の機能がないからですよね。
なんでないんでしたっけ？・・・分からんですよね。

視点を変えて、CMemoryにメモリ縮小の機能がない理由が分かったとします。
何か理由があって、メモリ縮小をしない設計になってるとします。
CMemoryの設計が理由で CNative::shrink_to_fit() を実装できない、という事実は、
CMemoryの設計を変える動機にはならないでしょうか？

・・・という方向に議論を展開させたほうがいいのか、
このPRのように CNative::Clear() の意味を変える方向で進んだほうがいいのか迷っていました。

一人で考えてても仕方ないと判断したので、上記別案を共有しました。

[beru]

shrink_to_empty は元の Clear 実装なんですね。

shrink_to_empty より shrink_to_fit の方が使う側からするとうれしい気がします。shrink_to_empty だと空にする事前提なので使いどころが限られるかなと思います。shrink_to_fit は CMemory::swap を使えば比較的簡単に実装出来ると思います。

shrink_to_fit より ShrinkToFit の方が他のメソッドと記法が揃っていて良いと思います。そこまで現状統一されているわけでも無いですが揃えておいた方がまぁ良いかなと。どちらの記法が優れているかについて話し出すときのこたけのこ戦争になりそう＞＜

どうせなら CMemory クラスに ShrinkToFit を実装した方が良いかなと思います。private 継承なので CNative にも同じ名前のメソッドを用意する必要は有りますね。

[berryzplus]

この PR マージ後に #777 を rebase して、 CNative::Clear() でデータサイズを空にする処理を実装する予定。

ここに納得がいってないです。

CNative::Clear()を実体に即した名前に変更するのはよいです。

しかし、CNative::Clear()を廃止新設して内容を変えるのはおかしいと思います。

string::clear() は補助記憶用の内臓メモリを開放し、lengthを0にする関数です。
CNative::Clear() は内臓メモリを最小サイズにして、lengthを0にする関数です。
ざっくりと「同じことをする関数」と言えると思います。

厳密には同じことをしてるわけじゃないので、
CNative::Clear() を「より適切な名前」に変更するのは構わないと思います。

CNative::Clear() を「length に 0 をセットする関数」にするのは違うと思います。
標準ライブラリと同じ関数名を使う以上は、なるべく挙動を合わせるべきと考えるからです。

[berryzplus]

ヘビさんとラクダさんのどっちが好きか？については、どっちでもいいです。

caseの違いが大事なのはコンパイルするときだけで、
人間が読む分にはclear()でもClear()でも変わらんと思ってます。

なんか、懐かしいCMを思い出しました・・・　キリンさんが好きです。(ry

[beru]

CNative::Clear() を「length に 0 をセットする関数」にするのは違うと思います。
標準ライブラリと同じ関数名を使う以上は、なるべく挙動を合わせるべきと考えるからです。

https://en.cppreference.com/w/cpp/string/basic_string/clear

ここを読むと、Notes に、

Unlike for std::vector::clear, the C++ standard does not explicitly require that capacity is unchanged by this function, but existing implementations do not change capacity. This means that they do not release the allocated memory (see also shrink_to_fit).

と書かれてました。何やらC++標準規格ではメモリ解放しない事を明示的に要求はしてないけど既存の実装はメモリ解放してないみたいなんです。

現在の使用状況に合わせてメモリ解放してほしい場合用に shrink_to_fit が C++11 から用意されたのだと推測してます。ただし non-binding request なので、呼ばれたら使用状況に応じてメモリ解放をするように必ず実装をしなければいけないというわけでもないようです。何か言い方がややこしいですが、出来れば実装しておいてね、みたいな感じなのかと。
https://en.cppreference.com/w/cpp/string/basic_string/shrink_to_fit

---

思い付きですが、Clear 呼び出し時にメモリ解放するかしないかはデフォルト引数で設定するやり方でも良いような気がします。

なんて事を言うとまた議論が発散、脱線してしまいそうですが…。

標準ライブラリと同じ名前を使った場合に挙動を合わせた方が良いかは、まぁそれに特に反対する理由は無いです。ただし、そうしなければいけない、とは思いません。ヘッダファイルを見た時にヘビとラクダが延々と交互に並んでいたらまぁ副作用で腹筋の運動にはなると思いますｗ

[berryzplus]

なんと！ぼくの誤解ですね。

https://ja.cppreference.com/w/cpp/string/basic_string/clear
（現在のstd::string実装は capacity を変更しない、と書いてあります。）

vs2017とvs2013、vs2010でソースを確認しました。
メモリ解放は行っていませんでした。
clear()の機能から「メモリ解放機能を削る」への反対は撤回します。
このPRが終わったあと、 Clear() を再新設するのは問題ないと思います。

あとは、ShrinkToFit() を実現したい、に対して、
ToEmpty()を実装することへの違和感が残ってます。

[m-tmatma]

https://ja.cppreference.com/w/cpp/string/basic_string/clear
（現在のstd::string実装は capacity を変更しない、と書いてあります。）

vs2017とvs2013、vs2010でソースを確認しました。
メモリ解放は行っていませんでした。

だとしたら、別にこの PR 対応しなくていいんじゃないかと思います。

現状 CNative::Clear() が使われている箇所はすべてローカル変数なので
メモリを解放したあと、再確保するのは無駄です。(インスタンス自体すぐ消えるので)

そのため、単に #777 をマージしてして、その後に #776 をマージすればいいと思います。

その後で、ShrinkToFit() を追加実装する必要があるか検討すればいいと思います。

[berryzplus]

既存7箇所の呼出箇所について、これClear()する意味なくね？という感想を抱いたことは秘密です。
配列系独自クラスを使う前に clear() する習慣があったみたいなのでそれの延長だったんでしょうか・・・。

というわけで #777 を approve してきました。

[berryzplus]

この PR は不要になったと思っています。