-
Notifications
You must be signed in to change notification settings - Fork 168
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
CNativeW クラスの SetString メソッドに std::wstring_view 型を受け取るものを追加 #1734
base: master
Are you sure you want to change the base?
Conversation
✅ Build sakura 1.0.3933 completed (commit 07e122c14d by @beru) |
Kudos, SonarCloud Quality Gate passed! |
✅ Build sakura 1.0.3937 completed (commit 0d59e8cf3f by @beru) |
Draftのままですが、残課題とかありましたっけ? |
テスト未実施なのでDraftにしています。 |
@@ -78,6 +78,7 @@ class CNativeW final : public CNative{ | |||
//WCHAR | |||
void SetString( const wchar_t* pData, size_t nDataLen ); //!< バッファの内容を置き換える。nDataLenは文字単位。 | |||
void SetString( const wchar_t* pszData ); //!< バッファの内容を置き換える。 | |||
void SetString( std::wstring_view str ) { SetString(str.data(), str.length()); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str.empty()
が true
の場合(≒ str.length() == 0
のとき)に str.data()
をアクセスすると落ちたような気がしないでもないです。
このPRって続行します?
個人的に懸念は「修正によって既存コードがクラッシュする事態にならないか?」だけです。 CNativeW dist;
const wchar_t* pNull = nullptr;
dist.SetString(pNull); //←これがstd::wstirng_view型引数と解釈された場合、クラッシュしてしまう。 |
PR の目的
SetString
メソッドを呼ぶ際に、std::wstring
型のインスタンスのc_str()
メソッドでポインタを引数にして呼びだすと、CNativeW::SetString( const wchar_t* pszData )
の実装でstd::wstring_view
のコンストラクタで文字列の要素数を調べる処理が走ります。文字列の要素数の情報はstd::wstring
型の size や length メソッドで定数時間で取得できるので、それを使う方が効率が良いと思い変更しました。カテゴリ
PR の背景
PR のメリット
.c_str()
の記述が無くせるので記述が短くなります。PR のデメリット (トレードオフとかあれば)
特にないと思います。
仕様・動作説明
PR の影響範囲
テスト内容
テスト1
手順
関連 issue, PR
参考資料
https://cpprefjp.github.io/reference/string/basic_string/length.html
https://cpprefjp.github.io/reference/string_view/basic_string_view.html