4

私は数週間前に書いたコードを少し持っています (コードの目的はその構造ほど重要ではありません):

if (_image.Empty)
{
   //Use the true image size if they haven't specified a custom size
   if (_glyphSize.Width > 0)
      imageSize.Width = _glyphSize.Width //override
   else
      imageSize.Width = _image.GetWidth;

   if (_glyphSize.Height > 0) then
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height = _image.GetHeight
}
else
{
   //No image, but they can still override it with a custom size
   if (_glyphSize.Width > 0) then
      imageSize.Width = _glyphSize.Width
   else
      imageSize.Width = 0;

   if (_glyphSize.Height > 0)
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height := 0;
}

今夜それを調べていて、クリーンアップしていたときに、クリーンアップされたバージョンはもっと簡潔でなければならないことに気付きました:

//Figure out the final image width
if (_glyphSize.Width > 0)
   imageSize.Width = _glyphSize.Width
else if (not _glyph.Empty)
   imageSize.Width = _glyph.GetWidth
else
   imageSize.Width = 0;

//Figure out the final image height
if (_glyphSize.Height > 0)
   imageSize.Height = _glyphSize.Height
else if (not _glyph.Empty)
   imageSize.Height = _glyph.GetHeight
else
   imageSize.Height = 0;

注:コードを切り詰めて論理フローを裸にし、ソース言語を難読化しました。

最後に、ネストされたif's を取り、それらを反転させました。そうすることで、この短縮が可能になりました。私の質問は、将来これをどのように認識できますか?

より短いコードにリファクタリングできるコードを書いたという明確な兆候は何ですか?

数週間前に私が持っていた別の例は、権限チェックに似たものでした: ユーザーはアクションを実行できます:

  • 彼らが許可を持っていれば、彼らはそれを行うことができます
  • 権限はないが、オーバーライドが有効な場合

私は最初に次のようにコーディングしました：

if ((HasPermission || (!HasPermission and OverrideEnabled))
{
   ...do stuff
}

ifその節の論理条件は、ちょっと冗長に思えました。ブール代数のコースに戻って、それを単純化する方法を見つけようとしました。結局、私はそれを行うことができたので、真理値表を描くことになりました:

Permission  Override   Result
    0           0        0
    0           1        1
    1           0        1
    1           1        1

私が見ると、OR演算です。したがって、私のifステートメントは次のようになりました。

if (HasPermission  or OverrideEnabled)
{
   ...
}

これは明白で単純です。そして今、私はどうして最初からそれを見ることができなかったのだろうと思っています。

SO の質問に戻ります。コードのブロックに TLC が必要であることを認識するために、どのような兆候を探すことができますか?

4

4 に答える 4

2

私の頭の上から、Code Completeからのいくつかのガイドラインを次に示します。そんな方におすすめの本です。

  1. ブロック内のネストされた if-else ステートメントと繰り返しステートメント
  2. 長い for ループ
  3. 繰り返しの行/ステートメントまたは頻繁に使用される操作を関数に配置できます
  4. 何らかの理由でコード行を何度もコピーして貼り付けている場合

個別の数学が、今の if ステートメントの書き方に影響を与えていることがわかりました。通常、2 つのブロックで 2 つの同じ IF ステートメントを書いていることがわかります。その後、精神的な「因数分解」を行います。

于 2010-01-23T03:13:55.070 に答える
1

素晴らしい。今、私がこれを見たとき：

//Figure out the final image width
if (_glyphSize.Width > 0)
...

//Figure out the final image height
if (_glyphSize.Height > 0)
...

リファクタリングはまだまだあると思います。コードをメソッドに抽出することは、冗長なコードを排除する優れた方法であるだけではありません。コードを自己文書化するための優れた方法でもあります。

コードを次のように減らしたいと思います。

set_final_image_size

set_final_image_size とそのミニオンを次のように定義します。

def set_final_image_size:
  imageSize.Width = final_image_width;
  imageSize.Height = final_image_height;

def final_image_width:
  if (_glyphSize.Width > 0)
     return _glyphSize.Width;
  else if (not _glyph.Empty)
     return _glyph.GetWidth;
  else
     return 0;

def final_image_height:
  if (_glyphSize.Height > 0)
     return _glyphSize.Height;
  else if (not _glyph.Empty)
     return _glyph.GetHeight;
  else
     return 0;
于 2010-01-23T03:55:06.310 に答える
1

特にブール評価に関連して、ほとんどの (?) 最新の言語が遅延評価を実装していることは注目に値します。

つまり、「a」が真の場合、if(a)とif(a or b)は論理的および機能的に同等です。orインタープリターは、真の変数の後を見ると評価を停止します。aとbが変数の場合、これはあまり重要ではありませんが、それらが callable [eg if(a() or b())] の場合、が true のb()場合は評価されません。a

これをよく学ぶことで、多くのキーストローク (およびプロセッサ時間) を節約できます。

if(!userExists()):
    if(!createUser()):
        errorHandling()
    else:
        doStuff()
else: doStuff()

になる

if(userExists() or createUser()): doStuff()
else: errorHandling()
于 2010-01-23T03:28:08.680 に答える
0

幅と高さのロジックを分離したので、それが同一であることに気付きました。たとえば、クラスに追加するgetDimension(Direction direction)とどうなるでしょうか? setDimension(Direction direction, int length)今、あなたは持っています

if (_glyphSize.getDimension(direction) > 0)
   imageSize.setDimension(direction, _glyphSize.getDimension(direction))
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

ローカルを抽出すると、次のようになります。

length = _glyphSize.getDimension(direction);
if (length > 0)
   imageSize.setDimension(direction, length)
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

もう少し考えてみましょう:

length = _glyphSize.getDimension(direction);
if (length == 0 && !_glyph.Empty)
    length = _glyph.getDimension(direction);
imageSize.setDimension(direction, length);

少なくとも私の目には、これはかなり素敵に見え始めています.

于 2010-01-23T17:04:52.067 に答える