auto_ 系の関数を使う必要が無いところで使わないように変更 #1040

beru · 2019-09-15T08:56:31Z

PR の目的

目的はリファクタリングを行ってコードの読み易さを向上する事です。

カテゴリ

リファクタリング

PR の背景

sakura_core/util/string_ex.h にある、接頭辞が auto_ のオーバーロード関数を使う必要が無い箇所では使わないように変更しました。以前は TCHAR を使用していたので ACHAR と WCHAR てオーバーロードされた関数を使う意味が有りましたが TCHAR を使わなくなったので WCHAR 用の関数を直接呼ぶようにしました。

なお auto_ 系の関数はインライン関数なので、経由しなくなったとしてもパフォーマンスの改善にはならないと思います。

PR のメリット

不要になった間接コードを経由しなくなる事で記述が明確になると思います。

PR のデメリット (トレードオフとかあれば)

ワイド文字列を処理する関数名になじみが無い人にとっては関数名から処理の内容を読み取るのが難しく感じるかもしれません。

PR の影響範囲

動作に影響はないはずです。

関連チケット

#1034 #1038

_tcsncmp_literal _tcsistr_j _tcs_toupper

AppVeyorBot · 2019-09-15T09:11:27Z

✅ Build sakura 1.0.2234 completed (commit 106f9d046d by @)

berryzplus · 2019-09-15T09:31:19Z

sakura_core/CGrepAgent.cpp

+	const int nPathLen = wcslen( pszPath );
 	auto szPath = std::make_unique<WCHAR[]>(nPathLen + 1);
 	auto szTmp = std::make_unique<WCHAR[]>(nPathLen + 1);
 	auto_strcpy( &szPath[0], pszPath );


文字列長を取得して、メモリ確保して、メモリコピーしてますね。
std::wstring使ったらいいような気がしないでもないです。

sakura_core/CGrepAgent.cpp

berryzplus · 2019-09-15T09:38:09Z

sakura_core/CGrepAgent.cpp

 			X / X  : (H)FullPath
 */
-			auto pszWork = std::make_unique<wchar_t[]>(auto_strlen(pszFullPath) + auto_strlen(pszCodeName) + 10);
+			auto pszWork = std::make_unique<wchar_t[]>(wcslen(pszFullPath) + wcslen(pszCodeName) + 10);


この10ってなんでしたっけ？
というのをどっかで確認したいです。

使われている書式文字列的にはそこまで要らないとは思いますが余裕を持って + 10 してるんでしょうね。

berryzplus · 2019-09-15T09:43:22Z

sakura_core/dlg/CDlgFavorite.cpp

+						size_t nLen = wcslen(pRecent->GetItemText(i));
 						std::vector<WCHAR> vecPath(nLen + 2);
 						WCHAR* szPath = &vecPath[0];
 						auto_strcpy( szPath, pRecent->GetItemText(i) );


ここも std::wstring 使ったほうが直感的に書ける気がします。

auto_strcpy が生き残ってますね…

それよりループの内側でローカル変数でSTLコンテナを使ってるのが効率が悪そうで気になります…。

それはそれで対策がいりそうですが別件でよいと思っています。
どっかで対応いれたいな、というのを書いてみただけです。

sakura_core/macro/CMacro.cpp

berryzplus · 2019-09-15T11:17:38Z

sakura_core/plugin/CPluginManager.cpp

 		if( plugin ){
 			// 要検討：plugin.defのidとsakuraw.iniのidの不一致処理
-			assert_warning( 0 == auto_strcmp( plugin_table[iNo].m_szId, plugin->m_sId.c_str() ) );
+			assert_warning( 0 == wcscmp( plugin_table[iNo].m_szId, plugin->m_sId.c_str() ) );


ここ wstring::compare にしても良さそうですね。

basic_string アレルギーなので使わないでおきます。:mask:

では std::char_traits<WCHAR>::compare で、と言ってみたりして 😄

というか、ここに関していうと assert_warning ってなんでしたっけ？とか assert_warning する意義は？とかも気になるんですけどね。

完全に別件だと思ってますので対応は不要です。

assert_warning はデバッグ時に軽微な問題を拾いやすくするための仕組みのようです。

berryzplus · 2019-09-15T11:21:32Z

sakura_core/prop/CPropComMainMenu.cpp

 					//	何も設定しない（元のまま）
 				}
-				else if (auto_strcmp(ptdi->item.pszText, L"") == 0) {
+				else if (wcscmp(ptdi->item.pszText, L"") == 0) {


ここ、else if ( !ptdi->item.pszText[0] ) としても同じですね。

おそらくいっしょですね。

berryzplus · 2019-09-15T11:22:23Z

sakura_core/outline/CDlgFuncList.cpp

 		/* クラス名::メソッドの場合 */
 		if( NULL != ( pPos = wcsstr( pWork, L"::" ) )
-			&& auto_strncmp( L"operator ", pWork, 9) != 0 ){
+			&& wcsncmp( L"operator ", pWork, 9) != 0 ){


9ってなんでしたっけ？
というのはあとにします。

_countof(L"operator ") だと思います。

-1 が必要かも…。

berryzplus · 2019-09-15T11:22:42Z

sakura_core/outline/CDlgFuncList.cpp

 						++k;
 						// Klass::operator std::string
-						if( auto_strncmp( L"operator ", pWork + m, 9) == 0 ){
+						if( wcsncmp( L"operator ", pWork + m, 9) == 0 ){


9ってなんでしたっけ？
というのはあとにします。

_countof(L"operator ") だと思います。

-1 が必要かも…。

berryzplus · 2019-09-15T11:23:42Z

sakura_core/_main/CCommandLine.cpp

 			// 不正なファイル名のままだとファイル保存時ダイアログが出なくなるので
 			// 簡単なファイルチェックを行うように修正
-			if (_tcsncmp_literal(szPath, L"file:///")==0) {
+			if (wcsncmp_literal(szPath, L"file:///")==0) {


何度か書いた独自マクロってのはこれのことです。
この変更に関してはなんも問題なしと思います。

AppVeyorBot · 2019-09-15T14:11:17Z

✅ Build sakura 1.0.2236 completed (commit 25bc4a8c92 by @beru)

berryzplus · 2019-09-15T14:48:26Z

sakura_core/func/CKeyBind.cpp

 	else {
 		if( !GetDllShareData().m_Common.m_sMainMenu.m_bMainMenuKeyParentheses
-			  && (((p = wcschr( sName, sKey[0])) != NULL) || ((p = auto_strchr( sName, _totlower(sKey[0]))) != NULL)) ){
+			  && (((p = wcschr( sName, sKey[0])) != NULL) || ((p = wcschr( sName, _totlower(sKey[0]))) != NULL)) ){


修正を確認しました。

berryzplus

問題ないと思います。
一応ローカルビルドと簡単な動作確認は行いましたが、問題なく動いていそうです。

beru · 2019-09-15T15:04:02Z

@berryzplus さん
確認ありがとうございました。Merge します。
もし問題があったら別のPRで対処します。

auto_ 系の関数を使う必要が無いところで使わないように変更

beru added 19 commits September 15, 2019 16:55

auto_strlen を使う必要が無いところで使わないように変更

e789c60

auto_strcpy を使う必要が無いところで使わないように変更

4b7d343

_tcsistr を使う必要が無いところで使わないように変更

46f40e2

auto_memcpy を使う必要が無いところで使わないように変更

3b20a54

auto_strcpy_s を使う必要が無いところで使わないように変更

61d300a

auto_strncpy を使う必要が無いところで使わないように変更

6ee34cb

auto_strcat を使う必要が無いところで使わないように変更

c0d6521

auto_strcat_s を使う必要が無いところで使わないように変更

47ea505

auto_memcmp を使う必要が無いところで使わないように変更

4e364f4

auto_memicmp を使う必要が無いところで使わないように変更

9755786

auto_strnicmp を使う必要が無いところで使わないように変更

b405ea4

auto_stricmp を使う必要が無いところで使わないように変更

d7dd86e

auto_strstr を使う必要が無いところで使わないように変更

876faf1

auto_strchr を使う必要が無いところで使わないように変更

46fa080

auto_atol を使う必要が無いところで使わないように変更

96868a1

auto_memset を使う必要が無いところで使わないように変更

cdcca90

auto_strcmp を使う必要が無いところで使わないように変更

c1bd913

auto_strncmp を使う必要が無いところで使わないように変更

e6b8c2a

下記のマクロ廃止

239918f

_tcsncmp_literal _tcsistr_j _tcs_toupper

beru added the refactoring リファクタリング【ChangeLog除外】 label Sep 15, 2019

berryzplus reviewed Sep 15, 2019

View reviewed changes

レビュー指摘対応

2bcc3da

berryzplus reviewed Sep 15, 2019

View reviewed changes

berryzplus approved these changes Sep 15, 2019

View reviewed changes

beru merged commit 0925433 into sakura-editor:master Sep 15, 2019

beru deleted the autobacs branch September 15, 2019 15:04

beru mentioned this pull request Sep 16, 2019

文字列比較関数の第2引数に文字列リテラルを渡す場合は大きさを自動取得するテンプレート関数を使うように変更 #1042

Merged

m-tmatma added this to the v2.4.0 milestone Dec 29, 2019

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020

Merge pull request sakura-editor#1040 from beru/autobacs

897bab3

auto_ 系の関数を使う必要が無いところで使わないように変更

gorogoro123 mentioned this pull request Mar 12, 2025

StaticString class template 更新 #2019

Merged

auto_ 系の関数を使う必要が無いところで使わないように変更 #1040

auto_ 系の関数を使う必要が無いところで使わないように変更 #1040

Uh oh!

Conversation

beru commented Sep 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR の目的

カテゴリ

PR の背景

PR のメリット

PR のデメリット (トレードオフとかあれば)

PR の影響範囲

関連チケット

Uh oh!

AppVeyorBot commented Sep 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beru Sep 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Sep 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berryzplus left a comment

Choose a reason for hiding this comment

Uh oh!

beru commented Sep 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

beru commented Sep 15, 2019 •

edited

Loading

beru Sep 15, 2019 •

edited

Loading