PHPの書き方を効率的に書き換えたいです。

Question

######説明
日付の比較を行っています。
コード量を減らして可読性を上げたいのですが、皆さんでしたら以下のコードをどのように書きますか？

■条件
<両方とも値がある>
$dnw05_afterDate = '2017/02/01'
$dnw05_beforeDate = '2017/02/28'

<afterDateのみ値がある>
$dnw05_afterDate = '2017/02/01'
$dnw05_beforeDate = ''

<beforeDateのみ値がある>
$dnw05_afterDate = ''
$dnw05_beforeDate = '2017/02/01'

if(is_string($dnw05_afterDate) && $dnw05_afterDate !== ''){
				$query->where(function ($exp, $q) use($dnw05_afterDate,$dnw05_beforeDate){
					return $exp->addCase(
						[
							$q->newExpr()->lte('dnw05', $dnw05_beforeDate),
							$q->newExpr()->between('dnw05', $dnw05_afterDate, $dnw05_beforeDate),
							$q->newExpr()->gte('dnw05', $dnw05_afterDate),
						]
					);
				});
			}

######自分のレベルで考えられるコード

if(is_string($dnw05_afterDate) && $dnw05_afterDate !== '' && is_string($dnw05_beforeDate) && $dnw05_beforeDate !== ''){
 //両方に値がある場合の処理
}elseif{
・
・
・
}

と何と不恰好な方法しか考えられず、、、

#####追記
現状のコードから生成されるのがこちらのSQL文です。

WHEN dnw05 <= '2017/01/31' THEN 'SMALL' 
WHEN dnw05 BETWEEN '2017/01/31'  AND '2017/02/18' THEN 'MEDIUM' 
WHEN dnw05 >= '2017/02/18' THEN 'LARGE' 

2つとも値がある場合

WHEN dnw05 BETWEEN '2017/01/31'  AND '2017/02/18' THEN 'MEDIUM' 

afterDateのみ値がある場合

WHEN dnw05 <= '2017/01/31' THEN 'SMALL' 

beforeDateのみ値がある場合

WHEN dnw05 >= '2017/02/18' THEN 'LARGE' 

と値によって、出力を制御したいです。

Comments

[退会済みユーザー]

値がかけた場合に、どう言う結果を返したいのかが不明です。つまり仕様が提示されていません。

[ssk]

情報を追記いたしました。上記の追記でお分りいただけますでしょうか？

[退会済みユーザー]

場合分けが十分にできていませんね…

[ssk]

申し訳ございません。他の場合があるということですか？

[退会済みユーザー]

回答に何が不備なのかわかるように記載します。

[t_obara]

そもそも可読性が高い＝コード量が少ないとは限りませんよね？ 本来の目的はなんですか？ コードを圧縮したいのか、可読性をあげる・メンテナンス性を向上させるなどどれが一番のポイントになるのでしょうか。

[ssk]

コメントありがとうございます。可読性をあげる・メンテナンス性を向上することが一番の目的です。

Answer 1

可読性をあげる・メンテナンス性を向上する

とりあえず、fooクエリジェネレータ的な関数を用意して、２つの日付を入力とし、
クエリオブジェクトを返却するようにする
文字列でない場合は、関数に渡す前にチェック
＊両方とも空だったらどうするのかは自分で考えて

関数の中身は、普通にifで場合分けすれば十分解りやすいかと。

*1 クエリオブジェクトをどのようにメインクエリに渡すかはマニュアルを参照してください。
フレームワークごとに違うでしょう。
Cakeっぽいので、以下でしょうか。
https://book.cakephp.org/3.0/ja/orm/query-builder.html
*2 fooは実際に使う時の内容で適当に設定してください。

Comments

[ssk]

回答ありがとうございます。
今回は、yambejp様の方法で実装することにいたしました。

それでも、まだまだ改善の必要があるので
t_obara様の方法も検討いたします。

助言、ありがとうございました。

Answer 2

#条件の洗い出しを抜け漏れなく行ってください。

これをいい加減にしてしまうと、基本的にバグの元になります。

start(s)	end(e)	target(t)	求める結果	備考
2017-01-01	なし	2016-12-01	?	s > t
2017-01-01	なし	2017-01-01	?	s = t
2017-01-01	なし	2017-02-01	?	s < t
なし	2017-01-01	2016-12-01	?	e > t
なし	2017-01-01	2017-01-01	?	e = t
なし	2017-01-01	2017-02-01	?	e < t
2017-01-01	2017-01-01	2017-01-01	?	s = e = t
2017-01-01	2017-03-01	2017-01-01	?	t = s < e
2017-01-01	2017-03-01	2017-03-01	?	s < e = t
2017-01-01	2017-03-01	2016-12-01	?	t < s < e
2017-01-01	2017-03-01	2017-02-01	?	s < t < e
2017-01-01	2017-03-01	2017-04-01	?	s < e < t

Comments

[ssk]

言葉足らずで申し訳ございませんでした。
参考テーブルまでありがとうございます。

[退会済みユーザー]

言葉足らずと言うより、こう言う条件を洗い出すというか、プログラミング脳になっていないのだと思います。訓練しかないとは思いますが…努力するしかないですよね。

[ssk]

はい＞＜努力いたします。

[退会済みユーザー]

頭の中がとっちらかっていたら、綺麗なコードって、絶対にかけませんからね…
さすがにこれくらいの場合分けがあると、プログラミング歴10年以上の自分でもこうして書き起こして情報を整理しないと、コードは書けません。

Answer 3

試す環境がないのですが

[

$q->newExpr()->lte('dnw05', $dnw05_beforeDate),
$q->newExpr()->between('dnw05', $dnw05_afterDate, $dnw05_beforeDate),
$q->newExpr()->gte('dnw05', $dnw05_afterDate),

]

の箇所がこうなればよいのでは？

[
    if($dnw05_afterDate==''){
      $q->newExpr()->lte('dnw05', $dnw05_beforeDate);
    }elseif($dnw05_beforeDate==''){
      $q->newExpr()->gte('dnw05', $dnw05_afterDate);
    }else{
      $q->newExpr()->between('dnw05', $dnw05_afterDate, $dnw05_beforeDate);
    }
]

修正

ごめんなさい、あきらかにおかしかったです。
これでどうでしょう？

[
  call_user_func(function() use($dnw05_afterDate,$dnw05_beforeDate){
    if($dnw05_afterDate==''){
      return $q->newExpr()->lte('dnw05', $dnw05_beforeDate);
    }elseif($dnw05_beforeDate==''){
      return $q->newExpr()->gte('dnw05', $dnw05_afterDate);
    }else{
      return $q->newExpr()->between('dnw05', $dnw05_afterDate, $dnw05_beforeDate);
    }
  })
]

Comments

[ssk]

ありがとうございます。

上記コードを試したのですが
syntax error, unexpected 'if' (T_IF), expecting ']'

が出てしまいました。

[yambejp]

明らかにおかしかったです、修正版を一度ためしてみてください