12

最近、誰かが私のコードを見て、手続き型すぎるとコメントしました。明確にするために、それは彼らが見たコードの多くではありませんでした-アプリケーションで取られた論理的なステップを明確に概説するセクションだけです。

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap())) {
    loadDataSources();
    initEngine();
    loadLiveData();
    processX();
    copyIds();
    addX();
    processY();
    copyIds();
    addY();
    pauseY();
    resumeY();
    setParameters();
}

次に、これらのさまざまなメソッドは、さまざまなオブジェクトの全体を作成し、必要に応じてこれらのオブジェクトに対してさまざまなメソッドを呼び出します。

私の質問は-手続き型プログラミングを示す、このようなアプリケーションを明確に駆動するコードのセクションです。もしそうなら、同じ結果を達成するためのよりOOな方法は何でしょうか？

すべてのコメントは大歓迎です！

4

8 に答える 8

8

ええと、このコードが存在するクラスには明らかにあまりにも多くの責任があります。ファサードにすべてのものを隠すことはしませんが、いくつかのftpエンジン、データソース、およびその他のエンティティに関連するすべてのものを単一の神オブジェクト（アンチパターン）に配置する代わりに、ビジネスプロセスが必要です。これらすべての種類のエンティティ。

したがって、コードは次のようになります。

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap())) {
    datasource.load();
    engine.init();
    data.load();
    engine.processX(data);
    data.copyIds()
    foo.addX();
    engine.processY();
    // ...
}

データソース、エンジン、およびその他すべてのコンポーネントがビジネスプロセスに注入される可能性があるため、a）テストがはるかに簡単になり、b）実装の交換が簡素化され、c）コードの再利用が可能になります。

手続き型のコードは必ずしも悪いとは限らないことに注意してください。

class Person {
    public void getMilk() 
    {
        go(kitchen);
        Glass glass = cupboard.getGlass(); 
        fridge.open(); 
        Milk milk = fridge.getMilk(); 
        use(glass, milk);
        drink(glass);
    } 
    // More person-ish stuff
}

そのコードは明らかに手続き型に見えますが、問題ない場合があります。ここで何が起こるかが完全に明確になり、ドキュメントは必要ありません（martinのクリーンなコードはそのようなコードを奨励します）。単一責任の原則と他のすべての基本的なOOPルールを念頭に置いてください。

于 2011-05-13T23:04:59.003 に答える
6

わお ！OOスタイルのようには見えません。このようにどうですか：

    ConnectionData cData = new ConnectionData(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap());


    if(downloadFeeds(cData)) {
      MyJobFacade fc = new MyJobFacade(); 
      fc.doYourJob();
    }

MyJobFacade.java 

public class MyJobFacade {

    public void doYourJob() {
          /* all your do operations maybe on different objects */
    }
}

ちなみにファサードパターン http://en.wikipedia.org/wiki/Facade_pattern

于 2011-05-13T22:47:07.260 に答える
6

私の質問は-手続き型プログラミングを示す、このようなアプリケーションを明確に駆動するコードのセクションです。

このコードフラグメントが「手続き型すぎる」かどうかを判断することはできません。

  • これらの呼び出しはすべて、現在のオブジェクトのインスタンスメソッド、個別のオブジェクト、または現在のインスタンスのインスタンス変数のいずれかを操作するためのものである可能性があります。これらは、少なくともある程度、コードをOOにします。

  • これらのメソッドがstaticの場合、コードは実際に手続き型です。これが悪いことであるかどうかは、メソッドがstaticフィールドに格納されている状態にアクセスして更新しているかどうかによって異なります。（名前から判断すると、おそらくそれを行う必要があります。）

もしそうなら、同じ結果を達成するためのよりOOな方法は何でしょうか？

コードの残りの部分を見ずに言うのは難しいですが、画像にはいくつかの暗黙のオブジェクトがあるように見えます。例えば

  • データソースおよび（多分）データソースマネージャーまたはレジストリ
  • engineある種の
  • ライブデータ、XとYを保持するための何か
  • 等々。

これらのいくつかはおそらくクラスである必要があります（まだクラスでない場合）...しかし、どれがクラスであるか、それらがどれほど複雑であるかなどによって異なります。（何らかの理由で）クラスとして意味をなさない状態は、静的変数をインスタンス変数に変換することで「よりOO」にすることができます。

他の回答は、特定のリファクタリング、すべてのグローバルの除去、依存性注入の使用などを提案しています。私の考えでは、これらの提案が役立つかどうかを判断するには情報が不十分です。

しかし、それは価値がありますか？

アプリケーションを「もっとOO」にするだけでは、有用または価値のある目標ではありません。コードをより読みやすく、保守しやすく、テスト可能で、再利用できるようにすることを目的とする必要があります。OOを使用することで問題が改善されるかどうかは、コードの現在の状態、新しい設計とリファクタリング作業の品質によって異なります。OOプラクティスを採用するだけでは、不適切な設計を修正したり、「悪い」コードを「良い」コードに変えたりすることはできません。

于 2011-05-14T01:22:47.000 に答える
4

私はこれを批評するために、それが「手続き的すぎる」かどうかとは異なるアプローチを取るつもりです。お役に立てば幸いです。

まず、関数パラメーターや戻り値が表示されません。これは、おそらくすべての種類のグローバルデータを使用していることを意味します。これは、必要に応じてここで読むことができる多くの正当な理由から避ける必要があります。グローバル変数は悪いですか。

次に、エラーチェックロジックが表示されません。例外でresumeYが失敗したとしましょう。問題はresumeYにあるかもしれませんが、pauseYの上位、またはloadDataSourcesまでの上位にある可能性があり、問題は後で例外としてのみ発生します。

これが本番コードであるかどうかはわかりませんが、いくつかの段階でリファクタリングするのに適しています。最初の段階では、各関数がブール値の成功を返すかどうかを確認し、各関数の本体で既知のエラーケースをチェックします。エラーチェックを行った後、関数argsを渡して結果データを返すことにより、グローバルデータの削除を開始します。失敗した場合に関数にnull値を返すようにするか、例外処理に変換することができます。例外をお勧めします。その後、個々のピースを個別にテスト可能にすることを検討してください。たとえば、データ処理機能とは別にdownloadFeedsをテストしたり、その逆を行ったりすることができます。

いくつかのリファクタリングを実行すると、コードをモジュール化して改善できる明らかな場所がわかり始めます。IMOは、OOPが十分であるかどうかについて心配する必要はなく、1。効果的にデバッグする、2。効果的にテストする、3。そのままにして、6か月後に戻って保守することで理解できるかどうかについて心配する必要があります。 。

この返信はかなり長くなりました。その一部がお役に立てば幸いです。:-)

于 2011-05-14T04:11:02.987 に答える
3

はい。メソッドは値を返さないため、スニペットからはグローバル変数を操作しているように見えます。教科書の手続き型プログラミングのようです。

よりOOのアプローチでは、次のようなものが表示されると思います。

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap()))
{
  MyDatasources ds = loadDataSources();
  Engine eng = initEngine(ds);
  DataObj data = loadLiveData(eng);
  Id[] xIds = processX(data.getX());
  Id[] newXIds = xIds.clone();
  data.addX(newXIds);
  Id[] yIds = processY(data.getY());
  Id[] newYIds = yIds.clone();
  data.addY(newYIds);
  pauseY();
  resumeY();
  someObject.setParameters(data);
}

ポール

于 2011-05-13T22:55:00.180 に答える
2

これが私が差別化することを学んだ方法です：

コードが「手続き型」になっている兆候は、クラスに複数の責任がある場合です（単一責任の原則に関するこのリンクを参照してください）。これらのメソッドはすべて1つのオブジェクトによって呼び出されているようです。つまり、1つのクラスが一連の責任を管理しています。

より良い方法は、これらの責任を、それらを最も適切に処理できる実際のオブジェクトに割り当てることです。これらの責任が適切に委任されたら、これらの機能を効率的に実行できるソフトウェアパターン（ファサードパターンなど）を実装します。

于 2011-05-13T22:56:55.983 に答える
1

これは確かに手続き型プログラミングです。あなたがそれをもっとOOにしようとしているなら、私はこれらの組み合わせを試してみます：

  • 保持しているデータを表すクラスを作成してください。たとえば、フィードを処理するFeedReaderクラス、データをロードするDataLoaderクラスなどがあります。
  • メソッドをまとまりのある機能を持つクラスに分けてみてください。たとえば、resume（）、pause（）を前のFeedReaderクラスにグループ化します。
  • process（）メソッドをProcessManagerクラスにグループ化します。

基本的に、あなたのプログラムをあなたのために働いているたくさんの従業員がいると考えてみてください、そしてあなたは彼らに責任を割り当てる必要があります。（PMが私のためにこれを行い、次にDMがその結果でこれを行います）。あなたが一人の従業員にすべての責任を与えるならば、彼または彼女は燃え尽きるでしょう。

于 2011-05-13T23:00:37.147 に答える
0

私はそれがあまりにも手続き的に見えることに同意します。手続き型に見えないようにするための明白な方法の1つは、これらすべてのメソッドをクラスの一部にすることです。Erhanの投稿は、それを分割する方法についてのより良いアイデアを提供するはずです:-)

于 2011-05-13T22:48:47.710 に答える