ちょっと気になるコード集
コードレビューしていた中で、気になるソースがあったので挙げてみる。たいしたことではないが、気になってしょうがない。
- Null許容型の使い方
- Nullチェック
int? inputValue = null;
// NG こんな実装方法は思いつかなかったので、少しだけすごいと思ってしまった。
if (string.IsNullOrWhiteSpace(inputValue.ToString()))
{
}
// OK プロパティを使う
if (!inputValue.HasValue)
{
}
// OK !Nullable.HasValueと等価なため、これでもよい。
if (inputValue == null)
{
}
2. Null許容型からの値取り出し。
int? inputValue = 1;
int value;
if (inputValue.HasValue)
{
// NG 何でも文字列にすれば良いという考えは止めよう
value = int.Parse(inputValue.ToString());
// NG 無駄なキャスト1
value = Convert.ToInt32((object)inputValue);
// OK プロパティを使用
value = inputValue.Value;
// OK 無駄なキャストに見えるが、inputValue.Valueが呼ばれる。
value = (int)inputValue;
}
- Object型
- キャスト
object objectValue = 1;
int castValue;
if (objectValue != null)
{
// NG 文字列にしてキャストするのは無駄が多い
castValue = int.Parse(objectValue.ToString());
// OK .Net Frameworkで用意された変換処理を使う
castValue = Convert.ToInt32(objectValue);
// OK 通常のキャスト
castValue = (int)objectValue;
}
まとめ
文字列にすれば良いと考えている人が多い。
動作としては問題なくても、レビューする側は気になってしょうがない。
サンプルコード(Java) 2017/3/24 追記
/** Nullチェック **/
Integer inputValue = null;
// NG javaでこのような書き方をする人はいないだろう。
// ちなみに、C#のint?はNullable構造体のため、ToStringする前のNullチェックは不要だった。
if (inputValue == null || inputValue.toString().length() == 0) {
}
// OK Javaの場合、nullで比較するのが正しい。
if (inputValue == null) {
}
/** Null許容型 **/
Integer inputValue = 1;
int value;
if (inputValue != null) {
// NG 何でも文字列にすれば良いという考えは止めよう
value = Integer.parseInt(inputValue.toString());
// NG 無駄なキャスト
value = (int) inputValue;
// OK Integer.intValueが呼ばれる。
value = inputValue;
// OK
value = inputValue.intValue();
}
/** Object型 **/
Object objectValue = 1;
int castValue;
if (objectValue != null) {
// NG 元の型が分かっている場合、文字列に変換する必要がない。
castValue = Integer.parseInt(objectValue.toString());
// OK 元の型へキャスト
castValue = (int) objectValue;
}