2009-02-27 7 views
5

私は、コマンドボタンのキャプションを状態変数として使用するために、私の最後の質問(How to gracefully exit from the middle of a nested subroutine when user cancels?)で正当な重要なフィードバックを受けました。私はそれが効率的なので、一度に2つか3つの目的に少しでも役立ちますが、私はそれが問題を引き起こす可能性があることを理解しています。VB6でボタンキャプションを変数として使用するのはどうですか?

私はこれが独自の議論に値すると思うので、ここではちょっと整理して "正しい"ように修正しました(基本的には文字列を単一の場所に定義してコードが失敗しないようにしています単にコマンドボタンのテキストを変更しただけです)。変数とコントロールの命名規則が悪い(OK、存在しない)ので、事前に申し訳ありません。しかし、私は状態変数の議論としてキャプションに集中しておきたいと思います。

は、だからここに私達は行く:

' Global variables for this form 
Dim DoTheThingCaption(1) As String 
Dim UserCancel, FunctionCompleted As Boolean 

Private Sub Form_Initialize() 
    ' Define the possible captions (is there a #define equivalent for strings?) 
    DoTheThingCaption(0) = "Click to Start Doing the Thing" 
    DoTheThingCaption(1) = "Click to Stop Doing the Thing" 

    ' Set the caption state when form initializes 
    DoTheThing.Caption = DoTheThingCaption(0) 
End Sub 

Private Sub DoTheThing_Click() ' Command Button 

If DoTheThing.Caption = DoTheThingCaption(0) Then 
    UserCancel = False ' this is the first time we've entered this sub 
Else ' We've re-entered this routine (user clicked on button again 
    ' while this routine was already running), so we want to abort 
    UserCancel = True ' Set this so we'll see it when we exit this re-entry 
    DoTheThing.Enabled = False 'Prevent additional clicks 
    Exit Sub 
End If 

' Indicate that we're now Doing the Thing and how to cancel 
DoTheThing.Caption = DoTheThingCaption(1) 

For i = 0 To ReallyBigNumber 
    Call DoSomethingSomewhatTimeConsuming 
    If UserCancel = True Then Exit For ' Exit For Loop if requested 
    DoEvents ' Allows program to see GUI events 
Next 

' We've either finished or been canceled, either way 
' we want to change caption back 
DoTheThing.Caption = DoTheThingCaption(0) 

If UserCancel = True Then GoTo Cleanup 

'If we get to here we've finished successfully 
FunctionCompleted = True 
Exit Sub '******* We exit sub here if we didn't get canceled ******* 

Cleanup: 
'We can only get to here if user canceled before function completed 

FunctionCompleted = False 
UserCancel = False ' clear this so we can reenter later 
DoTheThing.Enabled = True 'Prevent additional clicks 

End Sub '******* We exit sub here if we did get canceled ******* 

だからそれがあります。まだ実際に何かがありますかこのようにするのは悪いですか?それはちょうどスタイルの問題ですか?これらの4つのことをより望ましい、あるいは維持可能な方法で私に与えてくれる何か他のものがありますか?

  1. ユーザーのボタンを押してアクションがユーザーのためのワンボタンウェイ
  2. を希望されていない場合は、ユーザーの目にはすでにキャンセルする方法である場所でのアクション
  3. インスタントGUIからのフィードバックをもたらしたことをインスタントGUIのフィードバック(GUI上の混乱の量を減らす)操作
  4. をキャンセル/複数のクローズ要求

を防止するためのシンプルな、即時のコマンドボタンの無効化を開始するために私は1つの懸念はトンであるかもしれない見ることができます彼はコードとGUIの間のカップリングを(いくつかの方法で)閉じているので、大きなプロジェクト(または少なくとも大規模なGUI)では大きな問題になる可能性があります。これは、この種の「治療」を受けるボタンが2つまたは3つしかない小規模のプロジェクトです。

+0

UIを削除すると、IMOも同じように動作します。ユーザーと対話するのはそこだけです。 – Deanna

答えて

2

キャプションテキストと処理状態を切り離す方がよいと思います。また、gotoは読むのが難しい。ここに私のリファクタリングのバージョンがある...

Private Const Caption_Start As String = "Click to Start Doing the Thing" 
Private Const Caption_Stop As String = "Click to Stop Doing the Thing" 

Private Enum eStates 
    State_Initialized 
    State_Running 
    State_Canceled 
    State_Completed 
End Enum 

Private Current_State As eStates 

Private Sub Form_Initialize() 

    DoTheThing.Caption = Caption_Start 
    Current_State = State_Initialized 

End Sub 

Private Sub DoTheThing_Click() 

    If Current_State = State_Running Then 

    'currently running - so set state to canceled, reset caption' 
    'and disable button until loop can respond to the cancel' 
    Current_State = State_Canceled 
    DoTheThing.Caption = Caption_Start 
    DoTheThing.Enabled = False 

    Else 

    'not running - so set state and caption' 
    Current_State = State_Running 
    DoTheThing.Caption = Caption_Stop 

    'do the work' 
    For i = 0 To ReallyBigNumber 
     Call DoSomethingSomewhatTimeConsuming 

     'at intervals check the state for cancel' 
     If Current_State = State_Canceled Then 
     're-enable button and bail out of the loop' 
     DoTheThing.Enabled = True 
     Exit For 
     End If 

     DoEvents 

    Next 

    'did we make it to the end without being canceled?' 
    If Current_State <> State_Canceled Then 
     Current_State = State_Completed 
     DoTheThing.Caption = Caption_Start 
    End If 

    End If 

End Sub 
6

この手法で最大の問題は、文字列をブール値として使用することです。定義によれば、ブール変数は2つの状態しか持たないが、ストリングは任意の数の状態を持つことができる。

これで、コマンドボタンのテキストに許可された値を定義するために、定義済みの文字列の配列に頼ることで、これに内在する危険性を緩和しました。これはあまり問題のほんの一握りの葉:

  • Logicはより少なくより明示的な現在と利用可能な状態について(フォームのための4つの可能な状態が実際に存在している:-起動していない、開始、完了、開始 - しかし、キャンセル) - メンテナンスでは、ボタンテキストとブール変数状態の間の潜在的な相互作用を慎重に観察して、現在の状態が何であるべきかを判断する必要があります。単一の列挙によって、これらの状態が明示的になり、コードを読みやすく理解しやすくなり、メンテナンスが簡単になります。
  • コントロールプロパティ(ボタンテキスト)の動作に依存して、公開プロパティ値タイプ(文字列)との一貫性を維持しています。これは古いVB6コードをより新しい言語/プラットフォームに移行することを前提としています絶対地獄
  • 文字列の比較は、ブール変数の簡単なテストよりはるかに遅くなります。この場合、これは重要ではありません。一般的に、それを避けるのは簡単です。
  • DoEventsを使用してマルチスレッドをシミュレートしています(質問には直接関係ない...だが)。
+1

"DoEventsを使ってマルチスレッドをシミュレートしています" - 結局のところ* VB6です。 = P –

+0

ええ、VB6を嫌う理由はたくさんあります。 – Shog9

+0

マルチスレッドハード。簡単なDoEvents。 – MarkJ

5

この[ボタンキャプションを変数にする]の作業では、グローバリゼーションが悪夢であるということで、私は全体的に見てきました....私は古いvb6アプリケーションを英語とドイツ語...数週間ではないにしても数週間かかった。

あなたもgotoを使用しています...リファクタリングのビットはおそらくコードを読みやすくするために必要ですか?

**コメントへの応答で編集 私は各procの先頭にvb6でgotoを使用します。エラーが発生した場合は myErrorHandlerです。

次に、プロセスの最下部に、エラーをログに記録するためにグローバルハンドラにエラーを渡す1つのライナがあります。

+0

私は、1つの場所のgoto'sがOKだったと思っています。このようなエラートラップです。私の例では必要ではないことがわかりましたが、実際のコードにはキャンセルが発生するポイントがたくさんあります。そのため、いくつかのランダムなポイントで中断した後に残っているものをスキップするためには、フロー。 –

+0

ああ、私はここで悪夢のフラッシュバックをしています...数年単位を表す文字列を使った数年前の図書館で働いていました - "インチ" =インチなど。その後、ある時点で、 dはSI UOMsのサポートでハッキングされました... libは元のサイズの1/10でした。 – Shog9

2

別にDJがhis answerで行ったようにGOTOSの除去から、あなたのアプローチについては本当に何も悪いことはありません。ボタンキャプションには2つの状態しかありません。これらの2つの状態を使用してコード内のフローを定義します。

は、私は違っそれを行うだろう、なぜしかし、二つの理由があります:あなたは別の言語にあなたのプログラムを翻訳したいとき

  1. あなたの方法で問題を作成する(私の経験で、あなたべきそのため常に計画)キャプションが別の言語で変更されるためです。
  2. プログラムフローからユーザーインターフェイスを分離するという原則に反します。これはあなたにとって重要なことではないかもしれませんが、プログラムがますます複雑になると、ロジックからUIを明確に分離すると、作業がはるかに簡単になります。

解決策を手にしている場合は、ソリューションが確実に機能していて、なぜそうしてはならない理由はありません。しかし、一方で経験は、より複雑なプログラムでは、このやり方では少し異なるアプローチを使用することで簡単に回避できる問題を引き起こす可能性があることを教えてくれました。

また、あなたの例を批判した皆さんは、ある時点で類似の選択肢を作ったのだから、後でそれが間違いであることに気がついたと思います。

私は知っていました。

2

これは、基になるアルゴリズムをUIの特定の動作に結びつけます。さて、どちらか一方を変更したい場合は、両方を変更する必要があります。アプリのサイズが大きくなるにつれて、ロジックをカプセル化して変更をローカルに保存しないと、メンテナンスは悪夢になります。

1

何らかの理由でコードを操作する必要がある人は、使い慣れていて快適なプラクティスや慣習を見つけることができないため、機能の境界は存在しません。言い換えれば、Coupling/Cohesionトレイルで間違った方向に向かっています。 UIとの状態管理を機能的に統合することは、この問題の古典的なポスターの子供です。

あなたはOOPをまったく理解していますか?

5

あなたがそれらの問題を認識しているので、一般的なアーキテクチャ/カップリングの問題を無視して、あなたはそれらの問題を認識しているので、あなたのアプローチの1つの問題は、VB6コントロールがプロパティを設定するときに魔法のことをするためです。 あなたは単にプロパティを設定していると思うかもしれませんが、多くの場合イベントが発生する原因にもなります。チェックボックスの値をtrueに設定すると、クリックイベントが発生します。タブコントロールにtabindexを設定すると、クリックイベントが発生します。多くの場合があります。

私が正しく覚えていれば、プロパティを設定して直ちに読むとシナリオがあると思います。新しい値が表示される前に、画面のリフレッシュが発生していると思います。

私は、ストレージとしてコントロールプロパティを使用するあまりにも恐ろしいVB6コードを見てきました。この種のコードを見つけた場合は、リフレッシュメソッドDoEventsへの冗長な呼び出しで散在し、UIがハングアップすることがよくありますので、このコードを認識します。これは、プロパティが設定され、イベントが発生し、別のプロパティが設定され、最終的には最初のプロパティを再度更新するコード行を書き込む無限ループによって引き起こされることがよくあります。

これらの問題が十分にあなたを怖がらせていない場合は、これを考えてください。私たちの何人かは、それほど賢明ではありません。私はVB6で10年以上コーディングしており、おそらく約750KのLOCを個人的に書いており、私は上記の例を凝視し続けています。将来あなたのコードを読む必要があるすべての人々が、本当にシンプルなコードを書くことによって、本当にダムとなり、私たちを幸せにするだろうと仮定してください。

0

ローカライゼーションは、OPが提示しているロジックのタイプに最も大きな影響を与えます。何人かの人がそれを言いました - もしあなたが中国語にアプリを翻訳する必要があれば?そしてドイツ語?ロシア?

これらの言語をカバーする定数を追加する必要があります。純粋な地獄です。 GUIデータはGUIデータである必要があります。

ここでOPの説明は、Henry ford氏が言ったことを思い出させてくれました。「どんな顧客も、黒であればどんな色でも塗装することができます。

関連する問題