- VBAなんでも質問スレ
878 ：デフォルトの名無しさん[sage]：2014/09/13(土) 09:58:16.95 ID:/Kh9xLWL - >>868
すでにほかの方がおっしゃってるものも含めて私が個人的に感じたことです。
1.コード云々以前の問題として宣言されていない変数名が複数見受けられます。
Invoice_WS、 Setting_WS、 Input_WS、 Itemize、 Start_No、 End_No、は宣言無しで使われています。
（プロシージャ外で Public で宣言されているのかもしれませんが、その場合でも質問時には貼り付けておくべきです）
2.コメント内でのシート名の表記が統一されてないようです。
【工事一覧入力】、【一覧入力】、【入力一覧】、は
すべて Worksheets(Input_WS) を指しているようですから、どれかに統一した方が良いと感じました。
3.変数名のつけ方として、行を格納する変数なら Down ではなく Row、
列なら No ではなく Column に統一したほうが他人が見たときにわかり易いと思います。
また、見たところ Loop_Down も Loop_No も Worksheets(Input_WS) 内の情報だけを対象にしていますから
それも踏まえて Loop_Down は Input_Row、 Loop_No は Input_Column、のほうがよりわかり易いのではないでしょうか。
4.セル位置の指定方法として Cells(数字,"アルファベット") と Cells(数字,数字) が混在しています。
可読性の点や好みの問題もあるのですが、列指定にアルファベットを用いるのは
私はせいぜいA〜Zぐらいまでの範囲でしか使い物にならないと感じています。
今回はその範囲に収まっているのですが、列を変数で指定する事（たとえば貴方も Loop_No を使っています）も考えると、
Cells(数字,数字) のほうがよろしいかと思います。
5.Flagは一般的にはBoolean型変数を使います。
これらを踏まえて書き直してみました。
https://friendpaste.com/7iW6FESeuW3vqGQb1Xrb7I
勝手に付け足した部分には''で注釈を入れてます。
|
- VBAなんでも質問スレ
880 ：878[sage]：2014/09/13(土) 10:33:57.30 ID:/Kh9xLWL - >>879
そういうケースもあるのですね。
私の業務では行方向に0.1秒刻みとか1秒刻みで24時間分（86,400行〜864,000行）、
列方向に1日あたり複数の測定項目で1か月分（大体150列ぐらい）のデータが並ぶ、
といった帳票を処理するので、そういう場合には列指定がアルファベットだと仕事にならないわけです。
|
- VBAなんでも質問スレ
883 ：878[sage]：2014/09/13(土) 14:31:07.43 ID:/Kh9xLWL - ユーザーフォームを処理方法の分岐（全選択か一部選択か）に用いているようですが、
フォーム上から呼び出すプロシージャを変えるよりも、フォームで得られた値を返り値として
標準モジュール上で処理を分岐させるほうがコード全体の見通しが良くなる気がします。
通常、フォームモジュールと他のモジュール間での値の受け渡しにはパブリック変数を使うことが多いようですが、
変数はなるべく狭い範囲で使うに留めたいので、ユーザーフォームをUnloadしないでHideで制御を標準モジュールに戻します。
そうすると、フォームは見えないだけで内部の変数（Textboxの値など）は保持されていますので、
ユーザーフォームのオブジェクト名（今回ならfrm新発行確認）に続けてオブジェクトを指定すると値が取り出せます。
ということで、フォーム上に"発行開始請求書No."を入力する Textbox1、と
"発行終了請求書No."を入力するTextbox2、を用意して、フォームを閉じるボタンとしてToggleButton1 を用意します。
その他、Textboxに対する説明のLabelなどは適宜追加してください。
なお、フォームを閉じるのが CommandButton でなく ToggleButton なのは、キャンセル処理に対応するためです。
こうしておけばトグルボタンで閉じたときには処理続行、フォーム右上の×で閉じれば処理キャンセルという風にできます。
ということで、以下のコードは、>>882とほぼ同じ動作になります。
（開始終了の値の大小チェックや、数字以外の入力を受け付けないような処理が追加されてます）
https://friendpaste.com/1HbxD0LtNlfpEianBbT7wG
|
- VBAなんでも質問スレ
885 ：883[sage]：2014/09/13(土) 15:36:02.43 ID:/Kh9xLWL - >>883のコードですが、開始No.、終了No.ともに1以上である必要がありました。
よって正しくはこのようになります。
Private Sub TextBox1_Change()
'数字以外の入力は無視する
If Not IsNumeric(TextBox1.Value) Then
TextBox1.Value = ""
End If
If TextBox1.Value < 1 Then
TextBox1.Value = ""
End If
End Sub
Private Sub TextBox2_Change()
'数字以外の入力は無視する
If Not IsNumeric(TextBox2.Value) Then
TextBox2.Value = ""
End If
If TextBox2.Value < 1 Then
TextBox2.Value = ""
End If
End Sub
|