2011-11-14 12 views
1

基本的に大きなmain()関数で構成されるCコードを手渡しました。私は現在、メソッドをより小さな関数に展開し、コードの意図をより明確にしようとしています。でも、私はいくつかの問題を抱えています:手続き型コードのリファクタリング時のエラー処理

void main(int argc, char *argv[]) 
{ 
    if(argc != 3) 
    { 
     printf("Usage: table-server <port> <n_lists>\n"); 
     return; 
    } 
    int port = atoi(argv[1]), n_lists = atoi(argv[2]); 
    if(port < 1024 || port > 49151 || n_lists < 1) 
    { 
     printf("Invalid args.\n"); 
     return; 
    } 
    signal(SIGPIPE, SIG_IGN); 
    int sockfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); 
    struct sockaddr_in s_addr; 
    s_addr.sin_family = AF_INET; 
    s_addr.sin_port = htons(port); 
    s_addr.sin_addr.s_addr = htonl(INADDR_ANY); 
    if(bind(sockfd, (struct sockaddr *)&s_addr, sizeof(s_addr)) < 0) 
    { 
     printf("(bind).\n"); 
     return; 
    } 
    if(listen(sockfd, SOMAXCONN) < 0) 
    { 
     printf("(listen).\n"); 
     return; 
    } 

私はこのコードの機能は4つの主要な関心事を識別することができます:引数の数を確認することは正しいです

  1. を。
  2. コマンドラインからポートを取得します。
  3. 呼び出し信号(SIGPIPE、SIG_IGN)。
  4. 実際にソケットとの接続を試みます。

これを小さな関数にリファクタリングしようとすると、問題は主にエラー処理に関係します。

int verify_number_of_args(int argc) { 
    if (argc != 3) { 
     printf("..."); 
     return -1; 
    } 
    return 0; 
} 

、それが実際にその悪いわけではない。この

if (verify_number_of_args(argc) == -1) return; 

ようなものになるだろう呼び出す:例えば、1のロジックを抽出しようとしているrは次のようになります。さて、ソケットのために、それは両方のsockfds_addrを返却する必要があるような方法より面倒になるだろう、プラスのステータス戻り値:一種のよう私の主な方法を維持しようとの目的に反し

int sockfd; 
struct sockaddr_in* s_addr; 
if (create_socket(port, &sockfd, s_addr) == -1) 
    return; 

できるだけシンプルで明瞭です。私は、もちろん、.cファイルのグローバル変数に頼ることができましたが、それは考えの良いようではありません。

Cでこのようなことを一般的にどのように処理しますか?

+0

「エラー処理」タグを追加してタイトルを編集しました。 '[error-handling] [c]'のStackOverflowを検索してください。 –

+0

@キャットコール:ありがとう! –

答えて

3

ここに簡単なアプローチがあります。

引数の解析とそれに関連するエラーチェックはmainさんの関心事ですので、mainが極端に長い場合を除き、それらを分割しません。

それが適切に解析され、検証済みの引数を取ることを除いて、実際の作業は、プログラムのネットワーキング部分、すなわち、mainに非常によく似ている機能をオフに分割することができます。エラーについては

int main(int argc, char *argv[]) 
{ 
    // handle arguments 

    return serve(port, n_lists); 
} 

int serve(int port, int n_lists) 
{ 
    // do actual work 
} 

処理:このコードがライブラリではない場合、コールチェーンの深さにかかわらず、何か問題が発生したときに呼び出しプロセスを強制終了させることができます。それは実際には推奨される練習である(Kernighan &パイク、プログラミングの実践)。実際のエラー印刷ルーチンを、

void error(char const *details) 
{ 
    extern char const *progname; // preferably, put this in a header 

    fprintf(stderr, "%s: error (%s): %s\n", progname, details, strerror(errno)); 
    exit(1); 
} 

のように一貫性のあるエラーメッセージを得るために除外してください。 (LinuxやBSDでerr(3)をチェックし、他のプラットフォームでそのインタフェースをエミュレートしたいかもしれません。)

また、単純に間違っていない操作を除外するか、簡単に再利用可能なコンポーネントを作成するため、いくつかの不正な設定でシステムコールを呼び出すこともできます。

1

そのままにしますか?メインの開始時のセットアップの少しは問題を構成しません、IMO。物事が設定された後、リファクタリングを開始します。

1

リファクタリングのためにリファクタリングしているという印ではありませんか?

struct app_ctx { 
    int init_stage; 
    int sock_fd; 
    struct sockaddr_in myaddr; 
    ... 
} 

次にあなたがのインスタンスへのポインタを渡す:「のは一度に数sockfdとs_addrを初期化しましょう」について、あなたは常に 構造を作成することができ、それへのポインタを渡しとにかく

、この構造はすべてあなたの "一度に一つのことを"機能し、エラーコードを返します。

クリーンアップ時には同じことを行い、同じ構造を渡します。

+0

リファクタリングのためではありません。この関数は、基本的に絡み合ったifとelsesの150行の長さに似ています。 –

+0

@devouredelysiumが正しいかもしれないと思います。場合によっては、プログラムセグメントに名前を付けるためにいくつかの関数を導入すると便利な場合もあります。 –

関連する問題