c++ - この C++ コードを改善するにはどうすればよいですか

Question

次の疑似コードについて提案をお願いします。いくつかのデザインパターンを使用できるかどうかにかかわらず、どのように改善できるかを提案してください.


// i'm receiving a string containing : id operation arguments
data    = read(socket);
tokens  = tokenize(data," "); // tokenize the string based on spaces
if(tokens[0] == "A") {
   if(tokens[1] == "some_operation") {
      // here goes code for some_operation , will use the remaining tokens as arguments for function calls
   }
   else if(tokens[1] == "some_other_operation") {
     // here goes code for some_other_operation , will use the remaining tokens
   }
   ...
   else {
     // unknown operation
   }
}
else if(tokens[0] == "B") {
   if(tokens[1] == "some_operation_for_B") {
     // do some operation for B
   }
   else if(tokens[1] == "yet_another_operation") {
     // do yet_another_operation for B
   }
   ...
   else {
     // unknown operation
   } 
}

要点を理解していただければ幸いです。問題は、私には多数のid があり、それぞれに独自の操作があることです。また、多くのifとelse ifを含む 10 画面のコードがあるのはちょっと見苦しいと思います。

score 14 · Accepted Answer

共通のインターフェースを実装する各 ID のクラスを用意します。基本的に戦略パターンIIRC。

したがって、次のような（疑似）コードを呼び出します。

StrategyFactory.GetStrategy(tokens[0]).parse(tokens[1..n])

score 8 · Accepted Answer

まず、サポートする構文を書き留めてから、それをサポートするコードを書きます。

そのためには、BNF 表記を使用すると便利です。また、コード部分に Spirit ライブラリを使用するのは非常に簡単です。

Command := ACommand | BCommand

ACommand := 'A' AOperation
AOperation := 'some_operation' | 'some_other_operation'

BCommand := 'B' BOperation
BOperation := 'some_operation_for_B' | 'some_other_operation_for_B'

これは簡単に Spirit パーサーに変換されます。すべてのプロダクションルールはワンライナーになり、すべての終了記号は関数に変換されます。

#include "stdafx.h"
#include <boost/spirit/core.hpp>
#include <iostream>
#include <string>

using namespace std;
using namespace boost::spirit;

namespace {
    void    AOperation(char const*, char const*)    { cout << "AOperation\n"; }
    void    AOtherOperation(char const*, char const*)    { cout << "AOtherOperation\n"; }

    void    BOperation(char const*, char const*)    { cout << "BOperation\n"; }
    void    BOtherOperation(char const*, char const*)    { cout << "BOtherOperation\n"; }
}

struct arguments : public grammar<arguments>
{
    template <typename ScannerT>
    struct definition
    {
        definition(arguments const& /*self*/)
        {
            command
                =   acommand | bcommand;

            acommand = chlit<char>('A') 
              >> ( a_someoperation | a_someotheroperation );

            a_someoperation = str_p( "some_operation" )           [ &AOperation ];
            a_someotheroperation = str_p( "some_other_operation" )[ &AOtherOperation ];

            bcommand = chlit<char>('B') 
              >> ( b_someoperation | b_someotheroperation );

            b_someoperation = str_p( "some_operation_for_B" )           [ &BOperation ];
            b_someotheroperation = str_p( "some_other_operation_for_B" )[ &BOtherOperation ];

        }

        rule<ScannerT> command;
        rule<ScannerT> acommand, bcommand;
        rule<ScannerT> a_someoperation, a_someotheroperation;
        rule<ScannerT> b_someoperation, b_someotheroperation;

        rule<ScannerT> const&
        start() const { return command; }
    };
};

template<typename parse_info >
bool test( parse_info pi ) {
  if( pi.full ) { 
    cout << "success" << endl; 
    return true;
  } else { 
    cout << "fail" << endl; 
    return false;
  }
}

int _tmain(int argc, _TCHAR* argv[])
{

  arguments args;
  test( parse( "A some_operation", args, space_p ) );
  test( parse( "A some_other_operation", args, space_p ) );
  test( parse( "B some_operation_for_B", args, space_p ) );
  test( parse( "B some_other_operation_for_B", args, space_p ) );
  test( parse( "A some_other_operation_for_B", args, space_p ) );

    return 0;
}

score 4 · Accepted Answer

「Table Driven Methods」を調べることができます (「Code Complete」、第 2 版、第 18 章で説明されているように)。これがCheeryが説明していることだと思います。その利点は、拡張性が容易なことです。テーブルにいくつかのエントリを追加するだけです。このテーブルは、ハードコーディングすることも、実行時にロードすることもできます。

Epaga の提案と同様に、特殊なクラスにさまざまなケースでアクションを実行させることで、ポリモーフィズムを介してこれを解決することもできます。ここでの欠点は、変更があった場合に新しいクラスを作成する必要があることです。

score 2 · Accepted Answer

この問題の解決策として、うまく機能するものを見てきました。それは、関数のハッシュテーブルです。

コンパイル時に、サポートされている操作ごとに完全ハッシュ関数が作成され、操作は呼び出す関数に関連付けられます (関数ポインターはハッシュの値であり、コマンド文字列はキーです)。

実行時に、コマンド文字列を使用してコマンド機能が呼び出され、ハッシュテーブルで関数が検索されます。次に、参照によって「データ」文字列を渡して関数が呼び出されます。次に、各コマンド関数は、そのルールに従って残りの文字列を解析します...戦略パターンもこの時点で適用されます。

コードをステートマシンのように機能させます。これは (IMHO) ネットワークコードにアプローチする最も簡単な方法です。

score 2 · Accepted Answer

これを、ID ごとに 1 つ、操作ごとに 1 つの複数の関数に分割します。

私が通常使用するガイドラインは、画面の高さです。画面に機能を完全に収めることができない場合は、分割することを考え始めます。そうすれば、関数がどこに向かっているのかを確認するためだけにスクロールする必要がなくなります。先ほど言ったように、これはガイドラインであってルールではありませんが、構造を制御する方がより現実的だと思います。

その後、オブジェクト指向アプローチを採用して、これを一連のクラスに変換したい場合は、利点があれば、それを歓迎します。ただし、それに伴うすべての配管に注意してください。シンプルにしたいかもしれません。

デイブ

score 1 · Accepted Answer

関数のマップを作成します。次に、次のようなコードがあります。

consumed_count = token_mapper[tokens[0]](tokens)
remove amount of consumed tokens according to the return value and repeat.

とにかく、私はあなたのアプローチを理解していません.あなたは扱いにくく柔軟性のない言語を書くつもりです. 考えてみてください: 引数の量のわずかな違いが、その言語に大きな混乱を引き起こします。したがって、コマンドごとに常に 1 ～ 3 個の引数に制限されます。

レクサー/パーサージェネレーターの組み合わせを使用したいだけですが、やろうとしていることをしたい場合は、少なくとも最初に改行で分割し、次にスペースで分割することをお勧めします。したがって、 2 つまたは 3 つの引数を指定するためのものでした。

あなたの言語が機械で生成されたとしても、ジェネレーターにバグが発生した場合はどうなるでしょうか? 早期に失敗し、頻繁に失敗します。

score 1 · Accepted Answer

コマンドパターンを使用できます...各アクションはそのIDと操作を認識し、実行時にリストに追加します...その後、適切なコマンドを検索し、コンテキストに関係なく渡すだけです必要があり、操作を実行します。

score 1 · Accepted Answer

mxpが言ったように、テーブル主導のアプローチはこれに合っているようです。関数のパラメーターの数が異なる場合は、同じ行に関数のパラメーターの数を指定するテーブルの列を作成できます。

c++ - この C++ コードを改善するにはどうすればよいですか

8 に答える 8

Related

Reference