这是一个工作中遇到的背景比较简单的争论。

有这么一个persistent object,姑且叫它Plan吧。

有这么两个函数:

Plan getPlanByName(String userid, String planName);
Plan[] getPlans(String userid);


getPlanByName内部执行的是:
select * from Plan where userid=#userid# and plan_name=#planName#
and status=1
order by order_num

getPlan的内部执行的是:
select * from Plan where userid=#userid# and status=1


现在,我的pair认为这里面有DRY violation。因为两个select有些重复的东西。pair认为可以这样重构:
Plan getPlanByName(String userid, String planName){
  Plan[] plans = getPlans(userid);
  for(int i=0; i<plans.length; i++) {
    Plan plan = plans[i];
    if(planName.equals(plan.getPlanName())) {
      return plan;
    }
  }
  return null;
}


而我并不认为应该这样做。我的理由是:
1。原来的实现很简单直观。sql本来就是声明式语言。放着简洁的声明式不用而用复杂的命令式,有走回头路的嫌疑。
2。重构之后代码量更多,还要写更多的单元测试。
3。两个select只见的共同之处更像一种偶然的而不是概念上的重复。让getPlanByName依赖于getPlan感觉增大了耦合。
4。真要觉得select里面的东西有重复,不如创建一个view: v_plans这样这两个select就变成:
select * from v_plans where userid=#userid#
select * from v_plans where userid=#userid# and plan_name=#planName#

避免了status=1的重复。虽然还有"select *"之类的重复,但是这种重复就是语法上的,就像我们在java程序里面可能写无数次"static public void",我们从来不认为这是一个值得改变的重复。
5。效率。
6。系统本来是工作的。即使重构后确实好一点,也不值得花这个工作量。

而pair的主要观点是:
重构后维护上简单,不用维护两个select。

后来和组里其他人沟通,发现持重构想法的不只一个人,相对来说我的观点是比较孤立的。

那么你是怎么看这个问题?
评论
eivenchan 2007-08-28
在一些数据量比较少的查询中可能在性能上不会有太大的体现,
不过在100W+条记录就可以体现出来了....
现在我们做的项目大都使用存储过程过减少数据库和java之间的数据交互.

而且另外一个原因,
把简单的方法复杂化,不符合XP

支持楼主
hiwzg 2007-08-27
恩,我也觉得这个是不需要重构的。本来就是很简单明了的东西,你把他写到一块去,反而增加了逻辑判断。感觉和重构的本意相矛盾啊。
艾蛋蛋 2007-08-27
lane_cn 写道
无伤大雅的小问题,由他好了。
如果plan name不是数据库索引的话,在你的内存里面查和在数据表里面查实际上消耗的时间相差无几。
等他明白了软件应该怎样设计,他自然不会在这种小问题上多费一点脑筋。




Agree with your comments deeply.
hax 2007-08-24
ajoo同志胸中闷气终于抒发了出来。。。

不过你这个例子里,号称只是一个小数据集,又不涉及什么复杂逻辑,完全可以全部load到内存里。只是你pair的做法不伦不类,即要用数据库,又不好好用,确实是脑袋进水了。问题是你说公司其他人也多数赞同,难道你公司里其他人都脑袋进水?还是只是纯粹附和你的pair?
ajoo 2007-08-24
其实不仅仅是效率问题。我跟这种重用叫做ad-hoc reuse,对比于systematic reuse。

看,现在是有:
Plan[] getPlan(String userid);
Plan getPlanByName(String userid, String planName);


那么假如明天又有一个
Plan[] getPlanByPlanName(String planName);


这时候我们就有两种重用方案咯,getPlanByName既可以调用getPlan,也可以调用getPlanByPlanName。

回头如果再有一个
Plan[] getAllPlans();


是否最好getPlan(userid)和getPlanByPlanName()都要重构,通过调用getAllPlans然后再过滤呢?

一般化来说,如果有这么三个不同的条件判断:

select * from some_table where x and y and z

如果系统另外还需要
where x;
where y;
where x and y;
where y and z;
...

等等不同的变种,这个重用到底是谁重用谁?说穿了,最简单的,自然就是把整张表load到vm里,然后大家重用这一个,各自用循环来filer。可是这样你除了自己做了一个弱智无比的java数据库,什么也没干。一起工作时大家是同事,我不好意思刻薄,不过现在我终于忍不住要说,这就是脑袋进水了呀。
bulargy 2007-08-24
哎,不知道为什么要这么重构,感觉这样系统的负担更重了,本来可以一次查出来一条记录,这样重构后非要先查出N条再做筛选,感觉有点画蛇添足了!
caige_215 2007-08-23
对数据库方面代码的重用是不是要考虑性能方面.不要为了重用而重用
lszone 2007-08-15
典型的为了重用而重用
fixopen 2007-08-15
引用
Plan getPlanByName(String userid, String planName);   
Plan[] getPlans(String userid);

从逻辑上看,这两者似乎有密切的关系,由getPlans实现getPlanByName是合理的,也是说的通的。最初的实现就应该是如此的。

但是当我们落实到数据库这个底层支撑性的基础设施的时候,恐怕很多人【包括我】都会同意把采用getPlans的getPlanByName重构成另一个SELECT语句{注意:这儿说的是另一个方向的重构}。原因我觉得很明显,【数据库】这个基础设施提供了这个功能,而且所有的数据库都是如此,它们在上面花了无数的心血和精力实现的机制,为什么不用?这不是对DRY的公然否认吗?看起来这是对你坚持的“避免依赖”的肯定啊,哈哈。

如果觉得两个SELECT有问题,好,数据库层面为我们提供了灵活性,我们可以采用一个SELECT【+变量】的方式实现,这样就没有重复代码的坏气味了吧!

当然,这个争论其实不涉及到用户界面,所以骨子里是你们公司里给各种设施【包括数据库,应用服务器等等】【心理上】提供的权重问题。
抛出异常的爱 2007-03-05
lixigua 写道
逻辑在代码中就OO了,在SQL中就不OO了?
不愿意维护SQL,SQL是程序的魔鬼吗?在相当长的未来时间,我们还不得 不面对不怎么OO但很好用的SQL。


所以说DAO是装魔鬼的瓶子
封装它让它不能出了瓶子
lixigua 2007-03-05
逻辑在代码中就OO了,在SQL中就不OO了?
不愿意维护SQL,SQL是程序的魔鬼吗?在相当长的未来时间,我们还不得 不面对不怎么OO但很好用的SQL。
抛出异常的爱 2007-03-04
8844.43 写道
抛出异常的爱 写道
引用


Plan getPlanByName(String userid, String planName);
Plan[] getPlans(String userid);


getPlanByName内部执行的是:
select * from Plan where userid=#userid# and plan_name=#planName#
and status=1
order by order_num

getPlan的内部执行的是:
select * from Plan where userid=#userid# and status=1


多写一个方法
Plan[] getPlansOrige(Modle m)

里面的SQL句是:
select * from Plan where userid=#m.userid# and plan_name=#m.planName#
and status=1
order by order_num


你用到的两个方法引用这个方法得到的内容


PS:有时多易少,少弈多...

重构还是必要的,不管怎么说“只修改一处”的原则还是要坚持的。
那个pair的方法笨了些,“多写一个方法”就可以解决问题。
不过SQL语句恐怕还是要有两个的吧?!


少则少已,
只是小小的重复不用非要把SQL消灭干净才好
过度设计了.....
我所说的是把代码一样的部分提出来作一个方法
之后两个方法都去找这个提出来的方法
以达到不重复的作用...
但只是由于两次重复的话
我会用crtl +C ctrl+V 来作的....非常不值得我去作重构....

PS:在资源够用时可以进行这种重构
但是当资源不够时(多条SQL浪费了时间)请勿烂用....
ok_winnerboy 2007-03-04
我觉得重构不应该是一种赶时髦,更不应该是一种原则,而应该是每个程序员想"偷懒"时的一种自发行为.
lane_cn 2007-02-15
如果能把一张表相关的sql全部封装在一个类中,这样的封装程度就足够了,也比较容易做到。他非要全部封装在一个方法里面,没必要,也根本不可能。
BirdGu 2007-02-15
如果很多查询中都需要“status=1"这个条件, 那么用view应该是消除重复的最合理的方法。
李超群 2007-02-15
有点走火入魔了。怕重复的是逻辑。而这里本来就是两个不同的逻辑。
8844.43 2007-02-15
抛出异常的爱 写道
引用


Plan getPlanByName(String userid, String planName);
Plan[] getPlans(String userid);


getPlanByName内部执行的是:
select * from Plan where userid=#userid# and plan_name=#planName#
and status=1
order by order_num

getPlan的内部执行的是:
select * from Plan where userid=#userid# and status=1


多写一个方法
Plan[] getPlansOrige(Modle m)

里面的SQL句是:
select * from Plan where userid=#m.userid# and plan_name=#m.planName#
and status=1
order by order_num


你用到的两个方法引用这个方法得到的内容


PS:有时多易少,少弈多...

重构还是必要的,不管怎么说“只修改一处”的原则还是要坚持的。
那个pair的方法笨了些,“多写一个方法”就可以解决问题。
不过SQL语句恐怕还是要有两个的吧?!
techFurther 2007-02-15
我觉得这个问题没什么好争论的,对于具体的实现,每人都有其自己的实现路子,只不过你的pair觉得他这种方式更好一些,用一个理由来劝服你用这种方式而已。若是不用的话,你也可以找出充分的理由坚持你自己的做法哦。
hax 2007-02-14
ajoo同志,我觉得这个问题在于你们到底如何看待数据库。如果仅仅是一种比较方便的存储,那你同事的做法也可以,不过最好重构成上手就把所有的数据弄到内存里去,别让这个对象里有任何数据库调用的代码。

但是如果我是boss,花了n多钱买了个很贵的数据库,那当然会考虑数据库在整个架构中的运用,否则不是浪费嘛。说实话,我一直感到数据库sql还有存储过程很麻烦,因为会跟oo脱节,而且也有你的pair所痛感的重复代码问题。但是重复代码这样的问题要服从于架构!如果架构上决定(考虑到性能或者客户要求等各种因素)业务逻辑要充分利用sql甚至存储过程,那你pair的考虑就完全本末倒置了。

我听说一些项目,完全是用存储过程,java/c#的DAO都就是存储过程的包装代码。当然,对于我来说,我是很讨厌写这种代码的,那真成了软件蓝领了。我有一个同事在做一个.NET的项目时,用泛型和Attribute写了个代码生成器,自动从存储过程产生对应的C#代码。。。估计在这种架构中,这种方式是最好的了。

就你说的这个例子,总的来说,我不太赞同你的pair的做法。多维护一个sql又咋的啦?不必如此走火入魔啦。
strongkill 2007-02-14
后面的代码对数据库的成本高.每次都会改到不需要的数据.

倒不如用前面的代码.
ajoo
搜索本博客
最近加入圈子
存档
最新评论
  • SQL 小技巧
    第三个问题,先写出代码来吧。等有点时间再解释一下。第四个问题其实可以照猫画虎的: ...
    -- by ajoo
  • SQL 小技巧
    第一个问题是我在维护一个金融分析软件的时候碰到的。原来的那位老兄正儿八经地用一个 ...
    -- by ajoo
  • SQL 小技巧
    效率没问题。实际上一般的query效率都在查询上,至于对查询结果的计算,代价基本 ...
    -- by ajoo
  • Not Convinced about Java ...
    最讨厌所谓的魔法了,调试的时候能让人吐血。
    -- by aninfeel
  • SQL 小技巧
    ajoo 写道Readonly 写道问题一,经过google得到一用sum,lo ...
    -- by Readonly