2011-01-01 7 views
1

私はsomの本を読んで崇拝の音色をしましたが、それでもまだ良い流れのパターンがないと思っています。それはより多くのコーディングが必要になると思いますが、ここで長年のコーダーからsom pointerを得ることを願っています。コードフローの良い習慣?

iv 2つのサンプルを置いて、私がしたいことの1つの速い説明。そしてサンプル2は、それが何かに屈折しているが、右の経路にあるのかどうかわからない。私はEndCommand()を使う。方法かどうか?私はCloseStream()を使用しています。方法かどうか?そしてそのようなもの。

これはあなたの多くのための基本的なものですので、私のサンプルがひどいと間違っている場合私は許して願っていますが、私はもっと学びたいと思っています:)。

最後に、あなたはこれをどのように構築していますか?

サンプル1

using System; 
using System.Collections.Generic; 
using System.ComponentModel; 
using System.Data; 
using System.Drawing; 
using System.Linq; 
using System.Text; 
using System.Windows.Forms; 
using System.Diagnostics; 

namespace ConsoleGetTestWinform 
{ 
    public partial class Form1 : Form 
    { 
     private string _command = "tracert www.google.com"; 
     private string _application = "cmd"; 
     private string _exitCommand = "exit"; 

     public Form1() 
     { 
      InitializeComponent(); 
     } 

     private void button1_Click(object sender, EventArgs e) 
     { 
      richTextBox1.Text += GetTrace(); 
     } 

     private string GetTrace() 
     { 
      Process myprocess = new Process(); 
      System.Diagnostics.ProcessStartInfo StartInfo = new System.Diagnostics.ProcessStartInfo(); 
      StartInfo.FileName = _application; 
      StartInfo.RedirectStandardInput = true; 
      StartInfo.RedirectStandardOutput = true; 
      StartInfo.UseShellExecute = false; 
      StartInfo.CreateNoWindow = true; 
      myprocess.StartInfo = StartInfo; 
      myprocess.Start(); 
      System.IO.StreamReader SR = myprocess.StandardOutput; 
      System.IO.StreamWriter SW = myprocess.StandardInput; 
      SW.WriteLine(_command); 
      SW.WriteLine(_exitCommand); 
      string x = SR.ReadToEnd(); 
      SW.Close(); 
      SR.Close(); 
      return x; 
     } 
    } 
} 

サンプル2

using System; 
using System.Collections.Generic; 
using System.ComponentModel; 
using System.Data; 
using System.Drawing; 
using System.Linq; 
using System.Text; 
using System.Windows.Forms; 
using System.Diagnostics; 
using System.IO; 

namespace ConsoleGetTestWinform 
{ 
    public partial class Form1 : Form 
    { 
     private string _command = "tracert www.google.com"; 
     private string _application = "cmd"; 
     private string _exitCommand = "exit"; 

     Process process = new Process(); 
     ProcessStartInfo processStartInfo = new ProcessStartInfo(); 

     StreamReader streamReader; 
     StreamWriter streamWriter; 

     public Form1() 
     { 
      InitializeComponent(); 
     } 

     private void button1_Click(object sender, EventArgs e) 
     { 
      richTextBox1.Text += GetTrace(); 
     } 

     private string GetTrace() 
     { 
      StartProcess(); 

      string feedBack = streamReader.ReadToEnd();//reading output form cmd to feedBack 

      CloseStream(); 

      return feedBack; 
     } 

     private void StartProcess() 
     { 
      processStartInfo.FileName = _application; 
      processStartInfo.RedirectStandardInput = true; 
      processStartInfo.RedirectStandardOutput = true; 
      processStartInfo.UseShellExecute = false; 
      processStartInfo.CreateNoWindow = true; 
      process.StartInfo = processStartInfo; 

      process.Start(); 

      streamReader = process.StandardOutput; 
      streamWriter = process.StandardInput; 

      RunCommand(); //starting trace   
     } 

     private void RunCommand() 
     { 
      streamWriter.WriteLine(_command); 
      EndCommand(); //killing cmd 
     } 

     private void EndCommand() 
     { 
      //exiting cmd 
      streamWriter.WriteLine(_exitCommand); 
     } 

     private void CloseStream() 
     { 
      //ending stream 
      streamWriter.Close(); 
      streamReader.Close(); 
     } 
    } 
} 

答えて

1

私は最初の例を間違いなく好んでおり、他の回答はいくつかの点を強調しています。しかし、私は以下を追加したいと思います:

System.Diagnostics.ProcessIDisposableを実装します。 StreamWriterStreamReaderProcessインスタンスが所有します。したがって、プロセスが廃棄されると、それらのオブジェクトも閉じられ/廃棄される可能性があります。私はそれを検証するために実験したいと思いますが、.Dispose()を呼び出す限り、これらの2つのオブジェクトに.Close()を呼び出す必要はありません。すでに述べたように、プロセスオブジェクトにusingブロックを使用して、正しく処分されていることを確認してください(下記の例を参照)。

2番目の例では、ロジックの一部を分割しましたが、メンバー変数をフォームクラスに追加しました。これらの変数は、フォームの寿命が同じではありません。それは私の本の警告サインです。必ずしも悪くはありませんが、常に見直す価値があります。 2番目の例では、tracertの実行後にフォームがハングアップしている限り、デッドストリームがぶら下がります。また、ストリームを閉じてもプロセスを破棄しないので、破損したプロセスオブジェクトをそこに置くこともできます。

死んだオブジェクトへの参照にハングしないようにしてください(オブジェクトを使用すると変数をnullに設定できますが、最初の例よりエラーが発生しやすくなります)。彼らが分かりやすいように変数をスコープしてみてください。副次的なことに、もしそれらの変数が他のクラスに属していて、その平均寿命が包含クラス(例えばTracertクラス)と同じであれば、私はそれらがメンバであることに問題はないでしょう変数。

最初の例のGetTrace()メソッドは完全に満足していますが、分割する場合は、簡単な方法でメソッドprivate Process CreateProcess()を作成してProcessオブジェクトを作成して設定することができます。これは残りの作業を行うGetTrace()によって呼び出すことができます。

private Process GetProcess() 
{ 
    Process myprocess = new Process(); 
    System.Diagnostics.ProcessStartInfo StartInfo = new System.Diagnostics.ProcessStartInfo(); 
    StartInfo.FileName = _application; 
    StartInfo.RedirectStandardInput = true; 
    StartInfo.RedirectStandardOutput = true; 
    StartInfo.UseShellExecute = false; 
    StartInfo.CreateNoWindow = true; 
    myprocess.StartInfo = StartInfo; 
    return myprocess; 
} 

private string GetTrace() 
{ 
    using (Process myprocess = GetProcess()) 
    { 
     myprocess.Start(); 
     System.IO.StreamReader SR = myprocess.StandardOutput; 
     System.IO.StreamWriter SW = myprocess.StandardInput; 
     SW.WriteLine(_command); 
     SW.WriteLine(_exitCommand); 
     string x = SR.ReadToEnd(); 
     return x; 
    } 
} 

こうして、メンバ変数を追加する必要はありませんが、議論の対象となる可能性のあるものをより小さくすることができます。

+0

@Humphreyあなたのwtf/minはこの[提案](http://area51.stackexchange.com/proposals/11464/code-review?referrer=aWNm_PdciyFqjFW8CUacGw2 "コードレビュー")にとって非常に望ましいでしょう。サポートを表明し、ベータ版に手伝ってください。 :) – greatwolf

1

私は最初のサンプルを使用しますが、正確に行われているものを詳述し、いくつかのコメントを追加します。この方法は十分に小さく、私の意見ではリファクタリングを必要としません。また、StreamReaderStreamWriterのcloseメソッドを明示的に呼び出すのではなく、usingブロックを使用します。

大きな問題はメソッドではなく、Formのコードにあります。フォームに論理を持たず、そのメソッドとそれに属するすべてを独自のクラスに入れて名前を付けるべきですTracertのようなものです。

+0

oki私はクラスの問題に同意しました。もし私が大きなプログラムを作れば、私は屈折法が必要になる前にメソッドがどれくらい大きくなるのか考えていましたか? som text iv readは私に小さなメソッドを可能にしようとしていますが、私にとってはこれが常にreadabiletyを改善しないようです。 – Darkmage

+0

私は単語がないと思う、それは "resonable小さな方法"でなければなりません。一つの大きな方法しか持たないので、多くの小さな方法を持つことは間違っていて直感的ではありません。それにはレシピはありません。ケースに依存します。 – Femaref

2

ストリームは終了したらすぐに終了する必要があります。 常に閉じていることを確認するには、usingステートメントを使用します。

using (FileStream stream = File.Open ("the.file")) 
{ 
    //Do work on 'stream' 
} 

詳細は、メインUIスレッドでReadToEndのようなブロッキングメソッドを呼び出すために悪い考えです例えばDoes Stream.Dispose always call Stream.Close (and Stream.Flush)

1

を参照してください。プロセスが完了するまでに時間がかかる場合、GUIはハングアップします。

GUIを作成しているので、イベントベースのアプローチを使用することをお勧めします。あなたのプロセスのOutputDataReceivedイベントを購読してください。