多くのコーディング標準では、グローバル変数は極力使用しないようにしてくださいと説明されています。
グローバル変数が推奨されない主な理由には次のようなものがあります。
- どこからでも値を変更できてしまう
- 値が変更されたことを検知できない
- 値を変更する複数の処理を同時に実行できない
それぞれについてリファクタリング案を考えてみます。
どこからでも値を変更できてしまう
まず、読み取り専用でよいのではないですか？もしそうであれば、読み取り専用にしてください。値が変更される心配をしなくて済みます。.NET Framework ではフィールドに対して readonly を指定する方法と、get アクセサのみを定義した読み取り専用プロパティにする方法があります。
VB.NET
'読み書き可能なフィールド（いわゆるグローバル変数）
Public Shared MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
↓
'読み取り専用フィールド
Public Shared ReadOnly MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
'読み取り専用プロパティ
Public Shared ReadOnly Property MinimumDate As DateTime
Get
Return _MinimumDate
End Get
End Property
Private Shared _MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
C#
//読み書き可能なフィールド（いわゆるグローバル変数）
public static DateTime MinimumDate = new DateTime( 1900, 1, 1 );
↓
//読み取り専用フィールド
public static readonly DateTime MinimumDate = new DateTime( 1900, 1, 1 );
//読み取り専用プロパティ
public static DateTime MinimumDate {
get { return _MinimumDate; }
}
private static DateTime _MinimumDate = new DateTime( 1900, 1, 1 );
次に、実行時に値を設定できなくてはならない場合、アプリケーションのどこからでも値の設定が必要ですか？共通ライブラリの内部からのみ設定できればよい場合、Set アクセサのスコープを狭めることで実現できます。private を指定すればそのクラス内部からのみ設定可能、friend を指定すればそのクラスが定義されている共通ライブラリ内部からのみ設定可能となります。
VB.NET
'読み書き可能なプロパティ、但し設定はプロパティが定義されているクラス内部からのみ可能
Public Shared Property MinimumDate As DateTime
Get
Return _MinimumDate
End Get
Private Set ( value As DateTime )
_MinimumDate = Value
End Set
End Property
Private Shared _MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
C#
//読み書き可能なプロパティ、但し設定はプロパティが定義されているクラス内部からのみ可能
public static DateTime MinimumDate {
get { return _MinimumDate; }
private set { _MinimumDate = value; }
}
private static DateTime _MinimumDate = new DateTime( 1900, 1, 1 );
また、読み取り専用プロパティとは別に値設定メソッドを定義する方法もあります。
VB.NET
'読み取り専用プロパティ
Public Shared ReadOnly Property MinimumDate As DateTime
Get
Return _MinimumDate
End Get
End Property
Private Shared _MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
'値設定メソッド
Private Shared Sub SetMinimumDate( ByVal value As DateTime )
_MinimumDate = value
End Sub
C#
//読み取り専用プロパティ
public static DateTime MinimumDate {
get { return _MinimumDate; }
}
private static DateTime _MinimumDate = new DateTime( 1900, 1, 1 );
//値設定メソッド
private static void SetMinimumDate( DateTime value ) {
_MinimumDate = value;
}
ひとつ前の private な Set アクセサを定義する方法との違いは、インテリセンスなどで readonly として表示されることです。例え private であっても Set アクセサが定義されているとインテリセンス上では readonly とは表示されません。もちろん、どちらの場合もプロパティに対して値を設定しようとするとエラーにはなります。
値が変更されたことを検知できない
フィールドでは値が変更されたことを検知できませんが、プロパティの Set アクセサまたは値設定メソッドであれば検知できます。マイクロソフトのガイドラインに倣うと [プロパティ名]Changed というイベントを発生させるようにデザインするのが一般的です。
VB.NET
'読み書き可能なプロパティ、但し設定はプロパティが定義されているクラス内部からのみ可能
Public Shared Property MinimumDate As DateTime
Get
Return _MinimumDate
End Get
Private Set ( value As DateTime )
If ( _MinimumDate = value ) Then Return
_MinimumDate = Value
'値が変更されたことをイベントで通知
OnMinimumDateChanged( EventArgs.Empty )
End If
End Set
End Property
Private _MinimumDate As DateTime = New DateTime( 1900, 1, 1 )
'MinimumDateChanged イベントを発生させます。
Private Shared Sub OnMinimumDateChanged( e as EventArgs )
'Shared イベントであるためイベント発生元インスタンスは Nothing
RaiseEvent( Nothing, e )
End Sub
'MinimumDate プロパティの値が変更されたときに発生します。
Public Shared Event MinimumDateChanged As EventHandler
C#
//読み書き可能なプロパティ、但し設定はプロパティが定義されているクラス内部からのみ可能
public static DateTime MinimumDate {
get { return _MinimumDate; }
private set {
if ( _MinimumDate == value ) { return; }
_MinimumDate = value;
//値が変更されたことをイベントで通知
OnMinimumDateChanged( EventArgs.Empty );
}
}
}
private DateTime _MinimumDate = new DateTime( 1900, 1, 1 );
//MinimumDateChanged イベントを発生させます。
private static void OnMinimumDateChanged( EventArgs e ) {
EventHandler h = this.MinimumDateChanged;
//staic イベントであるためイベント発生元インスタンスは null
if ( h != null ) { h( null, e ); }
}
//MinimumDate プロパティの値が変更されたときに発生します。
public static event EventHandler MinimumDateChanged;
値を変更する複数の処理を同時に実行できない
このようなグローバル変数が使用される典型的な例は、ある共通関数で使用する値やその実行結果をグローバル変数に格納するというものです。これは難しい問題です。シングルスレッドアプリケーションであれば一時的に値を退避しておくなどの方法で回避できますが、マルチスレッドアプリケーションの場合はそれでは不十分です。率直に言うと、グローバル変数にすべきではありません。
関数で使用する値はその関数の引数にし、実行結果は戻り値または out 引数にしてください。そうすることによって多くの場合は問題を回避できます。本来、実行結果は戻り値として返し、戻り値を保持する必要があれば呼び出し側で保持すべきです。
対象の関数や変数が多かったり、呼び出し箇所が多くて関数の引数や戻り値を変更できない場合、次の手順でインスタンス化できないかを検討してみてください。例えば、データベース処理を行う共通関数 Execute があり、使用する値や実行結果がグローバル変数で定義されているとします。
VB.NET
Public Class DbUtility
'コマンドタイムアウトを取得または設定します。
Public Shared Property CommandTimeout As Integer
'エラーが発生したかどうかを取得します。
Public Shared ReadOnly Property HasError As Boolean
'最後に発生した例外を取得します。
Public Shared ReadOnly Property LastException As Exception
'指定されたSQL文を実行します。
Public Shared Function ExecuteSql( sql As String ) As Integer
End Class
C#
public class DbUtility {
//コマンドタイムアウトを取得または設定します。
public static int CommandTimeout {
get; set;
}
//エラーが発生したかどうかを取得します。
public static bool HasError {
get;
}
//最後に発生した例外を取得します。
public static Exception LastException {
get;
}
//指定されたSQL文を実行します。
public static int ExecuteSql( string sql );
これをアプリケーションで利用する場合、次のようなコーディングとなります。同じようなデータベース処理が同時に実行されている場合、コマンドタイムアウトや発生した例外が上書きされてしまう可能性があります。
VB.NET
'SQL文を生成
Dim sql As String = "insert into …"
'コマンドタイムアウトを変更
DbUtility.CommandTimeout = 100
'SQL文を実行
Dim affectedCount As Integer = DbUtility.Execute( sql )
'エラーが発生した場合はその内容を通知
If ( DbUtility.HasError ) Then
MessageBox.Show( "エラーが発生しました。"
& DbUtility.LastException.Message )
End If
C#
//SQL文を生成
string sql = "insert into …";
//コマンドタイムアウトを変更
DbUtility.CommandTimeout = 100;
//SQL文を実行
int affectedCount = DbUtility.Execute( sql );
//エラーが発生した場合はその内容を通知
if ( DbUtility.HasError ) {
MessageBox.Show( "エラーが発生しました。"
+ DbUtility.LastException.Message );
}
これをインスタンス化してみます。DbUtility クラスの中で関連する部分をコピーし、新たなクラスを定義します。プロパティやメソッドはインスタンスメンバに変更します（Shared/static を宣言しません）。新たなクラスとして定義するのは、DbUtility クラスを利用している他の処理に影響を与えないようにするためです。コンストラクタで接続文字列など必要な設定値を渡すようにするとよいと思います。
VB.NET
Public Class DbHelper
'コンストラクタ
Public Sub New()
'コマンドタイムアウトを取得または設定します。
Public Property CommandTimeout As Integer
'エラーが発生したかどうかを取得します。
Public ReadOnly Property HasError As Boolean
'最後に発生した例外を取得します。
Public ReadOnly Property LastException As Exception
'指定されたSQL文を実行します。
Public Function ExecuteSql( sql As String ) As Integer
End Class
C#
public class DbHelper {
//コンストラクタ
public DbHelper() {
}
//コマンドタイムアウトを取得または設定します。
public int CommandTimeout {
get; set;
}
//エラーが発生したかどうかを取得します。
public bool HasError {
get;
}
//最後に発生した例外を取得します。
public Exception LastException {
get;
}
//指定されたSQL文を実行します。
public int ExecuteSql( String sql );
}
グローバル変数の変更の衝突が発生する箇所のコーディングを変更します。DbUtility クラスを DbHelper クラスに置き換えます。但し、プロパティやメソッドはインスタンスメンバに変更されていますので、DbHelper クラスのインスタンス化が必要になります。
VB.NET
'SQL文を生成
Dim sql As String = "insert into …"
'インスタンスを生成
'通常使うインスタンスはアプリケーションの起動時などに
'あらかじめ生成しておくことが多いと思います。
'マルチスレッドで実行する処理など衝突が発生する場面では、
'通常使うインスタンスとは別にインスタンスを生成します。
'CreateDbHelper メソッドの内容は割愛します。
'個々のアプリケーションで実装するのではなく、
'共通関数としたほうがよいと思います。
Dim helper As DbHelper = CreateDbHelper()
'コマンドタイムアウトを変更
helper.CommandTimeout = 100
'SQL文を実行
Dim affectedCount As Integer = helper.Execute( sql )
'エラーが発生した場合はその内容を通知
If ( helper.HasError ) Then
MessageBox.Show( "エラーが発生しました。"
& helper.LastException.Message )
End If
C#
//SQL文を生成
string sql = "insert into …";
//インスタンスを生成
//通常使うインスタンスはアプリケーションの起動時などに
//あらかじめ生成しておくことが多いと思います。
//マルチスレッドで実行する処理など衝突が発生する場面では、
//通常使うインスタンスとは別にインスタンスを生成します。
//CreateDbHelper メソッドの内容は割愛します。
//個々のアプリケーションで実装するのではなく、
//共通関数としたほうがよいと思います。
DbHelper helper = CreateDbHelper();
//コマンドタイムアウトを変更
helper.CommandTimeout = 100;
//SQL文を実行
int affectedCount = helper.Execute( sql );
//エラーが発生した場合はその内容を通知
if ( helper.HasError ) {
MessageBox.Show( "エラーが発生しました。"
+ helper.LastException.Message );
}
こうすればプロパティに格納されている値はこの処理だけで使用するものになり、同時に実行されている処理との衝突は発生しません。
また、DbUtility が残ったままでは間違えて使用してしまう可能性があります。だからといって削除してしまうと既存のプログラムが動作しなくなってしまいます。残りの個所を置き換えるまでの間、ObsoleteAttribute 属性を用いて警告表示を行うのが一般的です。
VB.NET
<obsolete("同時処理による衝突を防ぐため、DbHelper クラスを使用してください。")> _
Public Class DbUtility
…
End Class
C#
[Obsolete("同時処理による衝突を防ぐため、DbHelper クラスを使用してください。")]
public class DbUtility {
…
}
ObsoleteAttribute は互換性のために残されている古いクラスやメソッドであることを利用者に知らせるためにライブラリ作成者が使用する属性です。.NET Framework 標準ライブラリの中にもこの属性が設定されているクラスやメソッドが多数存在しています。この属性が設定されているクラスやメソッドは引き続き利用することはできますが、Visual Studio 上に警告が表示されるようになります。
コードエディタで警告
0 件のコメント:
コメントを投稿