[ 初めての方へ | 一覧(最新更新順) | 全文検索 | 過去ログ ]
『Public変数について』(しのみや)
教えてください。
Sheet1 コマンドボタン 「CmdSTART_A」 コマンドボタン 「CmdSTART_B」
Sheet1のVBA Private Sub CmdSTART_A_Click() Call Sub作業1 End Sub Private Sub CmdSTART_B_Click() Call Sub作業2 End Sub
Module1 Public ws今月 As Worksheet Public Sub Sub作業1() Dim path今月 As String Dim wb今月 As Workbook Dim Lng最終行 as long path今月 = ThisWorkbook.path & "\Data\今月.xlsx" Set wb今月 = Workbooks.Open(path今月) Set ws今月 = FnCopySheet(target:=wb今月.Worksheets(1)) ws今月.Name = "今月" Lng最終行 = ws今月.Cells(Rows.Count, "A").End(xlUp).Row ’ 〜処理〜 msgbox "不要な行は削除してください。" End Sub
Private Function FnCopySheet(ByVal target As Worksheet) As Worksheet target.Copy Before:=ThisWorkbook.Worksheets(1) Set FnCopySheet = ActiveSheet End Function
Module2 Public Sub Sub作業2() Dim Lng最終行 as long Dim Str保存ファイル as string Lng最終行 = ws今月.Cells(Rows.Count, "A").End(xlUp).Row ’ 〜処理〜 ws今月.Copy Str保存ファイル = "今月.xlsx" ActiveWorkbook.SaveAs Filename:=ThisWorkbook.path & "\" & Str保存ファイル, _ FileFormat:=xlOpenXMLWorkbook Workbooks(Str保存ファイル).Close msgbox "終了しました。" End Sub
(*過去にtkitさんに教えて頂いたシートをコピーしてくるファンクションは 今回の質問のため短くさせてもらっています。 https://www.excel.studio-kazu.jp/kw/20210610095829.html)
CmdSTART_AとCmdSTART_Bを分けているのは、 AとBの間に使う人に作業を求める(不要な行を選択してもらって削除する)ためです。
【教えて頂きたいこと】 wb今月はファイルを開いて閉じるためだけですので、Dimで良いのですが ws今月はModule2でも指定したいのでpublicにしています…
動作は問題ないのですが、 こんな感じでおかしくないでしょうか… 何か違和感があるのですが、それが何か説明ができずにスミマセン…
< 使用 Excel:Excel2010、使用 OS:Windows10 >
期待どおりの動作をするなら、それで正解です。 って書けば満足するんですかね?
シート名が"今月"で決め打ちなので、Public変数で持つ必要もありません。 Public変数の寿命も有限です。 作業1と作業2の間に昼休みにでも突入して、ブック閉じたらどうなりますか? (´・ω・`) 2021/06/11(金) 17:41
今月、今月のオンパレードで勘違いしやしないかと緊張感が高まります。
今月ブックの今月シートだけしかないならまだ分かりますが、 そうと限った訳じゃないのでネーミングに工夫が必要と思います。
(半平太) 2021/06/11(金) 18:27
>Module2でも指定したいのでPublicに >何か違和感がある
プロジェクト全体を見渡した時、 「Sub作業1」と「Sub作業2」が別々のモジュールに設置されていることについては 感じない類の違和感でしょうか?
例えばの話、 Public変数を設置する場所が[Module1]でも[Module2]でもなく、 [Common]とか[General]とかいう名前の別の標準モジュールだったとしても、 その違和感って解消されませんか?
(白茶) 2021/06/11(金) 19:14
その違和感は、ロジックの組み方が分からない、といった事でしょうか。
今回の事例ならば、Public変数は使いません。 作業1、作業2が連続するコードであれば、まだいいですが、 「各々ボタンを押したら」ですよね。 (´・ω・`)さんの仰っている通りだと思います。
「CmdSTART_A」を押さずに「CmdSTART_B」を押したらエラーになりますよね? エラーを回避するために、ワークシートの存在確認をして存在したら そのワークシートを返すFunctionプロシージャを入れます。 それをプロシージャレベルの変数に入れればいいかと。 無かったら、何かしらのメッセージを表示して終了すればいいでしょう。
(tkit) 2021/06/14(月) 09:00
(´・ω・`)さん 半平太さん 白茶さん tkitさん ありがとうございます。
ここでPublic変数を使うことに対してなにかがおかしいな…は 作業1を押さずに作業2を押したケースが対応できていないから、 そして、Functionで存在確認をするとよいのですね。
いろいろわかりづらいなと思いながらも、ひとつひとつ進めていくので 精一杯です…ネーミングの管理もModuleの使い方も自分でも困っています -_-
Module2
Set ws今月 = FnExistsWorksheet("今月")
If ws今月 Is Nothing Then MsgBox "START_Aが押されていません。" Exit Sub End If
with ws今月 〜処理〜
'シートが存在するか存在しないか Function FnExistsWorksheet(ByVal Name As String) As Worksheet Dim ws As Worksheet For Each ws In Sheets If ws.Name = Name Then Set FnExistsWorksheet = ws ' 存在する Exit Function End If Next Set FnExistsWorksheet = Nothing End Function
FnExistsWorksheetのファンクションはModule2の下に書いていますが、 ほかのシートからも使う場合、Function用のModuleを作ったりするのでしょうか。
なにかおかしかったらまた教えてください。 (しのみや) 2021/06/14(月) 10:45
違和感の正体は回りくどさじゃないですかね。
今回はいわば「どんだけ"今月"が好きなの症候群」じゃないですか。
よしんば"今月"というのが実際には「R3_06」とかになるので変数にしておきたいということであっても、ブック名、シート、パスで逐一変数に格納する必要は全くないですよね。
それを踏まえると、それぞれ↓のようで問題ないようにおもいます。
Public Sub Sub作業1() Const 今月 As String = "今月" Dim Lng最終行 As Long
Workbooks.Open(ThisWorkbook.Path & "\Data\" & 今月 & ".xlsx").Worksheets(1).Copy _ Before:=ThisWorkbook.Worksheets(1)
With ThisWorkbook.Worksheets(1) .Name = 今月 Lng最終行 = .Cells(.Rows.Count, "A").End(xlUp).Row ' 〜処理〜 MsgBox "不要な行は削除してください。" End With
End Sub '======================================================================================= Public Sub Sub作業2() Dim Lng最終行 As Long
With ThisWorkbook.Worksheets(1) Lng最終行 = .Cells(.Rows.Count, "A").End(xlUp).Row ' 〜処理〜 .Copy End With
With Workbooks(Workbooks.Count) .SaveAs _ Filename:=ThisWorkbook.Path & "\" & .Worksheets(1).Name, _ FileFormat:=xlOpenXMLWorkbook .Close End With MsgBox "終了しました。" End Sub
(もこな2) 2021/06/14(月) 12:20
Sub 名無しのマクロ() Dim ws今月 As Worksheet On Error Resume Next Set ws今月 = ThisWorkbook.Worksheets("今月") On Error GoTo 0
If ws今月 Is Nothing Then MsgBox "START_Aが押されていません。" Exit Sub End If
With ws今月 '〜処理〜 End With End Sub
まぁ、[[20200924110137]]で「OnErrorで回避はなるべく使いたくない」と仰られていたので何かこだわりがあるのかもしれませんが・・・
(もこな2) 2021/06/14(月) 12:47
Set FnCopySheet = ActiveSheet
手動だと、確かにシートを新しいブックにコピーしたら、
新しいシートがアクティブになるんですが、
Set FnCopySheet = ActiveSheet
↑この行が実行されたとき、意図したシートが本当にアクティブになっているか、
分からないので、こういう書き方は怖いです。
(記憶では結構不安定な動作になるように記憶してます。)
Option Explicit
Private Sub CmdSTART_A_Click()
Call Sub作業1 End Sub
Private Sub CmdSTART_B_Click()
Call Sub作業2 End Sub
Private Sub Sub作業1()
Dim sTargetPath As String: sTargetPath = ThisWorkbook.Path & "\Data\今月.xlsx" Dim wbkOld As Workbook Dim wbkNew As Workbook Dim rngWork As Range
'ファイルの存在確認 If Dir(sTargetPath) = "" Then MsgBox "規定のファイルが存在しません。確認してください。" Exit Sub End If
'作業用ブック生成 Set wbkOld = Workbooks.Open(sTargetPath) wbkOld.Worksheets(1).Copy Set wbkNew = Workbooks(Workbooks.Count) '←ActiveSheetだと、今、コピーで出来たシートと担保されない wbkOld.Close False Set wbkOld = Nothing '閉じたブックのメモリーからの解放
'作業セル範囲の取得 Set rngWork = wbkNew.Worksheets(1).UsedRange
'〜セル範囲に対して作業
MsgBox "不要な行は削除してください。" End Sub
Private Sub Sub作業2()
Dim wbkTarget As Workbook Dim rngWork As Range
'規定のブックが開いているかチェック On Error Resume Next Set wbkTarget = Workbooks("今月.xlsx") On Error GoTo 0 If wbkTarget Is Nothing Then MsgBox "作業対象ファイルが開いていません。" Exit Sub End If
'セル範囲へ対しての作業 Set rngWork = wbkTarget.Worksheets(1).UsedRange '〜その他作業
'新規ブックへコピー rngWork.Worksheet.Copy
With Workbooks(Workbooks.Count) .SaveAs Filename:=ThisWorkbook.Path & "\" & "今月.xlsx", _ FileFormat:=xlOpenXMLWorkbook .Close False End With
MsgBox "終了しました。" End Sub
3)作業の前提となる条件のチェックが落ちています。
気になったのは、その3点かな。
(まっつわん) 2021/06/14(月) 15:05
画面をプロシージャ毎で表示したければ、
コードペインの左下にプロシージャの表示というボタンを押すといいかと思います。
表示するプロシージャは、右上のプルダウンメニューから選択します。
(まっつわん) 2021/06/14(月) 15:08
もこな2さん ありがとうございます。
私の頭の容量不足で情報が抜け落ちてしまっているのです。 たびたび教えて頂いているのにスミマセン
Worksheets(1)の状態で使う方法も進めてみました。
質問のコードでは短くさせてもらっておりますが、
ワークブックを開いて処理をし、 Sheets.Add.Name = "分析" でシートを作成する箇所がありまして、 ThisWorkbook.Worksheets(1)が"分析"に入れ替わってしまいます。
シートの動きがないコードはWorksheets(1)で、 シートが増えたり減ったりする場合はオブジェクト変数に入れておいたほうが良いのでしょうか…
「OnErrorで回避はなるべく使わないほうがよい」 こちらは前の会社の上司が言っていたことをその通りに覚えていただけでして、 その時は何かあっての発言だと思いますが、 今となっては気にしなくてよいことなのかなぁと思っております…
まっつあんさん ありがとうございます。 勉強させてもらいます。 今からですと明日になるかもしれません…ゆっくりですみません…
(しのみや) 2021/06/14(月) 15:26
どうやら今回の「違和感」は私の予想とは違ったみたいですので、 以下は本件が一段落ついた後にでも読み返して頂ければ程度の事ですが..(もうね、完全に無駄話ですから)
変数のスコープは、なるべく狭く設計するのがセオリーですよ。って事は頭の片隅に(一応)置いといた方がいいです。 (まぁセオリーなんて頑なに順守する程のものではないんですけど..^^;)
ループカウンタとしてお馴染み Dim i As Long の i さん。 たまにモジュールレベルで宣言されちゃってるのを見かけますが、「おいマジか...」って感じです。 (事務所に一つしかない穴開けパンチじゃあるまいし、ボールペンくらい自分で用意しようぜ的な^^;) それは余談ですけど、 変数を「見せる」「見せない」って、プロシージャのそれよりもシビアに統制すべきものだと思うんですよね。
例えば ある変数をUserFormと標準モジュールのどちらからも使いたい。って場合、 その変数を宣言する場所はUserFormか標準モジュールか、どちらで宣言しますか?
どっちでも出来ちゃうので「どっちでもいいよ」ってのも答えではあるんですが、 VBAで一番お世話になる[標準モジュール]って場所は、実はかなり特殊な場所なんだと思ってます。 これは 「その変数はUserFormに従属させるべきものですか? プロジェクト全体で共有されるべきものですか?」 という2択の問いに読み換えても良いと思います。 そう考えた時に「いや、どちらでもないぞ?」と感じるなら 「少なくとも標準モジュール上でPublic変数として宣言すべきではない」と考えてみます。
注意:「すべきではない」と申してるのではありません。一旦そう考えてみるってだけです。 何言ってんだコイツ...って思われる方もいらっしゃるとは思いますので。
そんな事に何の意味があるのか? ・・・実はあんまり無いケースの方が多いのですが^^; そのプロジェクトをメンテするハメになる将来の自分(あるいは後任者さん)にとっては 案外大きな意味を持っているかも知れません。
単純な話、 標準モジュールでPublic変数にしちゃうと、 他の標準モジュールで同じ名前の変数を宣言したい時に、 実際には宣言も区別も出来るんだけど、ただただ「紛らわしい」ですよね。
将来に渡って「見えなくても良いものまで見えちゃう状態」をなるべく回避しておこう。 という考え方です。
いやはや、全然関係無い話でスミマセンでした^^;
(白茶) 2021/06/15(火) 00:09
まっつわんさん 白茶さん ありがとうございます。
Functionでシートの有無を判断する方法と、 エラーで判断する方法どちらのも理解することができました。 教えていただいたことはそれだけではないですし、 わかったつもりがたくさんありそうなのですが… 自分で試して実感していきたいと思います。
>画面をプロシージャ毎で表示 こちらを知らなかったので、Moduleをわけていたのだと思います。
>狭く設計するのがセオリー わかっているのですがどうしたらよいかわからなくて Publicを使っていて違和感でしたが どうしたらよいかわからなかったです -_-
試してみるとまた行き詰って調べてまたわからないことになったりして… 頂いた情報を拾うのが必死で、たくさん抜け落ちていてスミマセン
Public Sub Sub作業1() Dim TargetPath As String Dim wb As Workbook Dim ws As Worksheet Dim Lng最終行 As Long TargetPath = ThisWorkbook.Path & "\Data\今月.xls" If Dir(TargetPath) = "" Then MsgBox "ファイルが存在しません。確認してください。" Exit Sub End If Set wb = Workbooks.Open(TargetPath) wb.Worksheets(1).Move Before:=ThisWorkbook.Worksheets(1) ’↑Openしたエクセルのシートが1枚なので、Moveでシートを移動してくるとエクセルをCLOSEする必要がない Set ws = Worksheets(1) Set wb = Nothing With ws .name="今月" 〜処理〜 '=============================== Public Sub Sub作業2() Dim ws as worksheet On Error Resume Next Set ws今月 = ThisWorkbook.Worksheets("今月") On Error GoTo 0 If ws今月 Is Nothing Then MsgBox "作業対象ファイルが開いていません。" Exit Sub End If
一度このような感じで進めてみようと思います。 なにかおかしかったらまた教えてください。 (しのみや) 2021/06/15(火) 10:58
まず、この「Sub作業1」と「Sub作業2」というのはSheet1だけから呼び出されるものでしょうか。
それとも、他のシートからも呼び出されることがあるのでしょうか。
もし、Sheet1だけしか使用しないということならSheet1のモジュールに置いておくのがベターだと思います。
変数のスコープはなるべく狭くというセオリーが出てましたが、プロシージャも同様のセオリーが適用できると思います。
Sheet1のモジュールに「Sub作業1」と「Sub作業2」を置いておけば、両方で共通で使う変数はSheetモジュールの先頭でPrivateで宣言できますので、スコープはモジュール内になります。
もし、複数シートから呼び出される共通処理ということなら、途中でユーザー操作が入るということで、いろいろ考慮すべきことが増えて複雑なことになりますので、その場合は、また、別の考え方になりますので、ご指摘ください。
あと、処理の途中でユーザー操作が入るという場合、今回なら「Sub作業1」→ ユーザー処理 →「Sub作業2」という順が保障される必要があります。
その場合は、自分なら下記のような設計にします。
CmdSTART_B のEnabledプロパティをFalseに設定しておきます。
Private Sub CmdSTART_A_Click() Call Sub作業1
CmdSTART_A.Enabled = False CmdSTART_B.Enabled = True End Sub
Private Sub CmdSTART_B_Click()
Call Sub作業2
CmdSTART_A.Enabled = True CmdSTART_B.Enabled = False End Sub
こうすれば起動直後は CmdSTART_A しか押せません。
CmdSTART_A を押したら、CmdSTART_B が押せるようになります。
また、CmdSTART_A は押せないようになります。
これで、CmdSTART_Aを押す前にCmdSTART_B を押す、CmdSTART_Aを続けて2度押す、CmdSTART_Bを続けて2度押す、というような操作ミスが防げます。
(hatena) 2021/06/15(火) 13:48
hatenaさん ありがとうございます。 Enabledプロパティの方法お教えくださりありがとうございます。
>この「Sub作業1」と「Sub作業2」というのはSheet1だけから呼び出されるものでしょうか。 Sheet1から呼び出されるだけのものです。
Sheet1モジュールに書いていくと Sheetを消された時にvbaも一緒に消えてしまったことがあって、 そのあたりから標準モジュールを使うようになってきたと思います… 曲がった言い訳のような気がしていますが…
なるべくセオリーに沿って作っていきたいので、 標準モジュールは共通処理を置くようにして、 Sheet1の作業はSheet1のモジュールに書きたいと思うのですが、 自分でも思わぬ時にシート削除してしまう可能性があり、 この場合シート削除をできないよう保護をかけておくような対応になるのでしょうか? それはそれで手間かな…ウーン
手持ちのVBAがまだぐちゃぐちゃなので…状況把握ができてきたら、 共通処理を標準モジュールにまとめてインポートして使えるようにしたりしようと思うのですが まだいろいろ追いついておりません…
各シートの作業毎にSubを分けたほうがよいのかとか… ピボット作成毎にSubを分けたほうがよいのかとか… 保存作業だけのSubにしたほうがよいのかとか… 分けすぎなのかとか… 変数設定は、必ず最初にしていたほうがよいのかとか… Application.ScreenUpdating = False や On Error GoTo Error1 など どこに置くかで悩んでいたりして なんだかいろいろ…
ちょっと質問になっていないのですが、 自分ならこうしている等ありましたらお聞かせいただけると助かります…
話がブレブレでスミマセン
(しのみや) 2021/06/15(火) 14:37
>自分ならこうしている等ありましたらお聞かせいただけると助かります…
私も同じ経験をしましたよ。 自分のコードは一般的なのかどうか悩んだことがありました。 引数で渡すことから、クラスモジュールでオブジェクト化するあたりで、 何度コードを書きなおしたことやら・・・
数か月前に書いたコードを修正するのに、まず何をどこでやっているか 読み解くのに大分時間が掛かったことから、可読性を意識するようになり、 他人が使うとバンバンエラーになることから、様々な想定からエラートラップを するようになりで、日々成長しているんだな、という気持ちで書き直していました。
可読性、処理速度、汎用性を天びんにかけながら、ご自身でルールを決めていくしかないと思いますよ。 どこぞのサイトでは、必ずSelectを入れて、SelectionやActive〜を使うことを推奨していたり・・・ 人それぞれなんですよね。
>自分でも思わぬ時にシート削除してしまう可能性があり、・・・
十分な理由ですので、ご自身のルールにしてしまいましょう。
私は、 ある程度の処理速度(最速100msec→1sec程度に遅くなった程度)であれば、可読性を重視します。 コード量が多少増えても可読性を重視します。 基本、標準モジュール、クラスモジュールに重要なコードは書いて、ブック、シートモジュールには 出来るだけコードは起きません。(イベント程度) プロシージャ名や変数名に2バイト文字は使用しません。(officeのバグが怖い) スコープ毎で変数に命名ルールを設けています。 他にいろいろありますが、こんなもんですかね。
(tkit) 2021/06/15(火) 15:46
Sheet1モジュールに書いていくと Sheetを消された時にvbaも一緒に消えてしまったことがあって、 そのあたりから標準モジュールを使うようになってきたと思います… 曲がった言い訳のような気がしていますが…
削除されることを心配するなら、そのシートをバックアップ用のブックに保存しておいて、削除されたり、破損したときに、バックアップからコピーしてくるという方法をとればいいかと思います。
シートを作成して、コマンドボタンを配置して、そのクリックイベントを記述するより手間が少ないと思います。
コード自体がまだ修正途中のようですし、仕様変更や機能追加でコード修正はいろいろ発生すると思います。
下手に修正してうまくいかなかたっときに以前の状態に戻したいということも結構あります。それに備えて、定期的に数世代のバックアップを取っておいて、依然の状態に戻せるようにしておくのもいいと思います。
特に大きなプロジェクトの場合は私はそうしてます。
(hatena) 2021/06/15(火) 16:08
>Sheet1モジュールに書いていくと >Sheetを消された時にvbaも一緒に消えてしまったことがあって、
なるほどなーと思う部分もありますが、
そもそもシートがない時点でほぼほぼ、アウトですよねー。
バックアップをどう取っておくかという話になるかと^^;
>各シートの作業毎にSubを分けたほうがよいのかとか… >ピボット作成毎にSubを分けたほうがよいのかとか… >保存作業だけのSubにしたほうがよいのかとか… >分けすぎなのかとか…
臨機応変にとしか言いようがないような。。。
伝わるか分かりませんが、
大まかに言ったときにまとめられる、
作業単位ごとにプロシージャを作ることになるかと。
>変数設定は、必ず最初にしていたほうがよいのかとか…
必ずではないですね。僕は最初にずらずら定義しておく方が好きですが、
最近は使う直前に定義するのが流行りのようです。
> Application.ScreenUpdating = False や On Error GoTo Error1 など
> どこに置くかで悩んでいたりして なんだかいろいろ… 必要最小限の範囲ですかね。 (まっつわん) 2021/06/15(火) 16:28
>「OnErrorで回避はなるべく使わないほうがよい」 >こちらは前の会社の上司が言っていたことをその通りに覚えていただけでして、 >その時は何かあっての発言だと思いますが、 >今となっては気にしなくてよいことなのかなぁと思っております…
やたらと、「On Error 〜」を使うと、デバッグ時にバグの箇所を探し当てることが、
困難になります。
事前にエラー回避の処置を書けるなら、そちらを優先すべきかと。
雑にさくっと書き上げたいときは限定的に最小限使うのはありでしょう。
のちのち、メンテナンスを誰かに引き継ぐならば、
丁寧に書かなければいけないかと思います。
(まっつわん) 2021/06/15(火) 18:49
> 「OnErrorで回避はなるべく使わないほうがよい」 > こちらは前の会社の上司が言っていたことをその通りに覚えていただけでして、 > その時は何かあっての発言だと思いますが、 > 今となっては気にしなくてよいことなのかなぁと思っております…
時と場合によりけりなんですよ。
その見極めがつかないレベルの人は「なるべく使わないのがいい」のであって 下のichinoseさんの様なレベルが高い人は有効活用するんです。
[[20150906185643]] On Error 〜ステートメントについて』(ichinose)
(半平太) 2021/06/15(火) 20:40
tkitさん hatenaさん まっつわんさん 半平太さん ありがとうございます。
頭がいっぱいで、今はお礼のみになってしまいますが…
整理しながら進めていきたいと思います。 またぼんやりとした質問をさせてもらうことがあるかと思いますが… お時間よろしければ書き込んでいただけると助かります。 (しのみや) 2021/06/16(水) 11:16
[ 一覧(最新更新順) ]
YukiWiki 1.6.7 Copyright (C) 2000,2001 by Hiroshi Yuki.
Modified by kazu.