##Getter、Setterを逆にするのは何故か
普通にコーディングに問題無い人でも、Getter、Setterを逆にするコードを見かける。取得すべきデータを戻り値としていればGetter、内部変数を変更したり、引数を更新する場合はSetterを使用すれば間違うことは無いと思うのだが、そう単純でもないのかもしれない。サンプルコードをみて考えてみる。
##サンプルコード
サンプルコードはいずれも、実際のプロジェクトで見たことがあるコードを簡略化している。
なお、NG1～NG3までの処理に関連はなく、一連の処理ではない。
private void Main()
{
// NG1 戻り値がなく、引数を更新しているのにGetメソッド
var dtoes = new List<Dto>();
GetData(dtoes);
// NG2 戻り値があるのにSetメソッド
dtoes = SetData();
// NG3 正しいSetterが混在していて読みにくい
var dto2 = SetDto2();
dto2.SetDtoes(SetData());
}
private void GetData(List<Dto> dtoes)
{
// DBから取得する想定。
dtoes.Add(new Dto());
}
private List<Dto> SetData()
{
// DBから取得する想定。
var dtoes = new List<Dto>();
return new List<Dto>();
}
private Dto2 SetDto2()
{
var dto2 = new Dto2();
return dto2;
}
/** テーブルのDTO **/
private class Dto
{
}
/** Dtoをメンバーに持つ親DTO **/
private class Dto2
{
public List<Dto> _dtoes;
public void SetDtoes(List<Dto> dtoes)
{
_dtoes = dtoes;
}
}
java
public void main() {
// NG1 戻り値がなく、引数を更新しているのにGetメソッド
List<Dto> dtoes = new ArrayList<Dto>();
getData(dtoes);
// NG2 戻り値があるのにSetメソッド
dtoes = setData();
// NG3 正しいSetterが混在していて読みにくい
Dto2 dto2 = setDto2();
dto2.setDtoes(setData());
}
private void getData(List<Dto> dtoes) {
// DBから取得する想定。
dtoes.add(new Dto());
}
private List<Dto> setData() {
// DBから取得する想定。
List<Dto> dtoes = new ArrayList<Dto>();
return new ArrayList<Dto>();
}
private Dto2 setDto2() {
Dto2 dto2 = new Dto2();
return dto2;
}
/** テーブルのDTO **/
private class Dto {
}
/** Dtoをメンバーに持つ親DTO **/
private class Dto2 {
public List<Dto> _dtoes;
public void setDtoes(List<Dto> dtoes) {
_dtoes = dtoes;
}
}
##考察
上記サンプルコードのようになった理由を考えてみる。
GetData:DBからデータを取得しているのでGetter
SetData:DBから取得したデータを設定して返しているのでSetter
SetDto2:インスタンスを設定して返しているのでSetter
SetDtoes:Dtoを設定しているのでSetter
というところだろうか。
統一感がなくで、コードが読みにくい。
取得するデータを戻り値とするならGet, 内部変数の更新や引数の更新をSetというルールで書き直してみた。
##リファクタリング後
private void Main()
{
// OK 引数更新
var dtoes = new List<Dto>();
SetData(dtoes);
// OK 戻り値が取得データになっている。
dtoes = GetData();
// OK GetterとSetterが統一されて読みやすくなった。
var dto2 = GetDto2();
dto2.SetDtoes(GetData());
}
private void SetData(List<Dto> dtoes)
{
// DBから取得する想定。
dtoes.Add(new Dto());
}
private List<Dto> GetData()
{
// DBから取得する想定。
var dtoes = new List<Dto>();
return new List<Dto>();
}
private Dto2 GetDto2()
{
var dto2 = new Dto2();
return new Dto2();
}
/** テーブルのDTO **/
private class Dto
{
}
/** Dtoをメンバーに持つ親DTO **/
private class Dto2
{
public List<Dto> _dtoes;
public void SetDtoes(List<Dto> dtoes)
{
_dtoes = dtoes;
}
}
java
public void main() {
// OK 引数更新
List<Dto> dtoes = new ArrayList<Dto>();
setData(dtoes);
// OK 戻り値が取得データになっている。
dtoes = getData();
// OK GetterとSetterが統一されて読みやすくなった。
Dto2 dto2 = getDto2();
dto2.setDtoes(getData());
}
private void setData(List<Dto> dtoes) {
// DBから取得する想定。
dtoes.add(new Dto());
}
private List<Dto> getData() {
// DBから取得する想定。
List<Dto> dtoes = new ArrayList<Dto>();
return new ArrayList<Dto>();
}
private Dto2 getDto2() {
Dto2 dto2 = new Dto2();
return dto2;
}
/** テーブルのDTO **/
private class Dto {
}
/** Dtoをメンバーに持つ親DTO **/
private class Dto2 {
public List<Dto> _dtoes;
public void setDtoes(List<Dto> dtoes) {
_dtoes = dtoes;
}
}