我写得 OOP 今天被喷了

• 请不要在回答技术问题时复制粘贴 AI 生成的内容

This topic created in 4385 days ago, the information mentioned may be changed or developed.

事情是这样的，原来的代码是这样：

$login = new ILogin();
$uid = $login->getLoginUid();
if ($uid < 10000) {
return json_encode(array('ret'=>-1));
}

我改成了这样：

$login = new ILogin();
$uid = $login->getLoginUid();
if ($login->hasError()) {
return json_encode($login->getErrorl());
}

说我没事瞎JB调用这么多函数干啥，明明可以一步到位的。
我解释：这样方便维护，而且语义强。
他说：你这个太多调用方法，速度慢。
我:.....

我就是喜欢OOP，但不知道这种情况改怎么反驳

Supplement 1 · Nov 7, 2014

统一回复下：

1.有人可能光看这一段代码去了。我现在设计的所有的model里面只要有false的，都会调用 :
$this->setError(errorCode,array('msg'=>'数据不存在'));

2.昨天有用xhprof测试过, 10000次，第二种是第一种的调用时长的2倍。

3.我一直觉得我的写法是对的。但对方掐我死穴，调用太多方法效率比起他那个稍显不足这一点我无法反驳。我弱弱的说我的方便维护，好像不起啥作用。我只是想请教各位遇到这种情况怎么说服别人。。。。。。

uid

调用

51 replies • 2014-11-08 18:28:05 +08:00

chmlai

Nov 7, 2014

速度慢~哈哈哈

pykwokcc

Nov 7, 2014

让我来写，我也会写成楼主的写法

yxz00

Nov 7, 2014

我倒是觉得原来那个版本好些。只不过把那个magic num改成login的属性maxid就好了。

yxz00

Nov 7, 2014

这个跟oop也没什么关系

haiyang416

Nov 7, 2014 via Android

从这几行代码里我没看出你拿 uid 做什么呀，第一个反倒很明白。
我始终觉得不是为了面向对象而面向对象，不是所有东西都需要封装。
这仅对你给的几行代码而言，团队项目的话一切按约定来，个人项目随便了。

sivacohan

Nov 7, 2014

我写成类似上面那个被喷了。
咱俩换公司把。

jemyzhang

Nov 7, 2014

国内很多都喜欢简单,而不考虑维护性.

记得之前在微信上绑定招行信用卡,硬是绑定不上,弹出错误说我没开卡.后来和客服妹妹过了N天的招,并且后台维护也介入了,还是没解决.

最后的最后的最后才发现原来是我没设定查询密码.

我只想说,去你妹的没开卡,错误信息是这样做的么?!

赞楼主~

fengliu222

Nov 7, 2014

无法评判这两段代码的好坏。
你写的那段，$uid压根儿没用上。
还有就是，把错误封装成一个方法hasError是否有必要，是要看错误码的多少，如果错误情况比较多，那么集中处理判断确实挺好的。如果只有一个，就有点设计过度了。。

PrideChung

Nov 7, 2014

如果是我就一定会用看小狗的眼神看着他说：“要不要给你介绍几本程序设计的入门书？”

patr0nus

Nov 7, 2014

搭配LZ头像看贴风味更佳;)

zakokun

Nov 7, 2014

我倒是觉得原来那个语义简介好理解....不过楼主的话配合你的头像很有喜感

hslx111

Nov 7, 2014

这个代码没有上下文，很难判断谁对谁错

cxshun

Nov 7, 2014

就一行调用还真没必要了。但以速度慢来反驳，只能说哈哈了。

leiz

Nov 7, 2014

如果< 10000和 ret -1那个用的地方很多，从维护的角度看，你的做法是对的。不然等以后维护的时候，接手的人哭死。
如果只是一两处，看喜好。我个人会偏向你的写法，否则以后要调整这些错误体系的话，折腾死。

chemzqm

Nov 7, 2014

OO的思路更适合功能复杂的产品，简单的调用更适合糙快猛的项目，如果需求多变项目，真没必要过度封装，只会增加所有人的理解成本。

fising

Nov 7, 2014

真是啥都能喷。。。

bingu

Nov 7, 2014

支持楼主派和反对楼主派梅花间竹地出现了。

levn

Nov 7, 2014

快，把json_encode也封起来

curiousjude

Nov 7, 2014

@fengliu222 楼主这只是一个代码片段而言，都hasError了，uid当然是没有啦。另外，错误码的多少不一定是一成不变的，系统复杂后，可能的错误情况也会增多。

tedeyang

Nov 7, 2014

1，用uid兼做错误代码也真是醉了。这种代码已经不是维护性差的程度，是二义性错误。
2，性能问题有个经典答案：如无必要，不需优化。优化了方法调用的几个纳秒对以ms计的HTTP网络传输有什么意义？要快请用opcode，用java，用C。
3，你的代码也很烂。model、validation、error不应该这么耦合。如果遇到允许为false的字段，你这个机制就出问题了。
这么喜欢OOP，还不转投java阵营？Spring、Annotation、Bean的设计在解决你这个问题上甩了几条街。。。

jox

Nov 7, 2014

OOP是啥谁来告诉我

RemRain

Nov 7, 2014

hasError 不是面向过程的方法么，OOP 的话，抛个异常吧

johnsneakers

Nov 7, 2014

@tedeyang 可能我这里给的例子不对，总之原来项目的代码全是如下这种：
contronller:
$obj = new Model();
$userinfo = array();
$ret = $obj->getUserInfo(&$userinfo); //这里引用传递,只是直观，语法错误请无视。
if($ret < 0)
{
return json_encode(array('ret'=>$ret));
}

另外我model里面没有写error方法哦，所有model都有父类， error方法都是在父类里面，这里不详细帖了。。。。请大神指教

loryyang

Nov 7, 2014

过早的性能优化是罪恶的源泉，特别是为了性能而去牺牲系统架构优雅性，增加模块耦合，降低代码可读性、易维护性等。当然，如果没有后面这些损失，你当然应该写更高效的代码。

很多时候你根本不会知道最影响性能的是哪部分代码。你过早的优化，也许只是优化了占总耗时1%的那部分代码。

关于你这个代码，我觉得原来的代码也还可以，oop不是解决问题的唯一方式，不用oop也一样可以写出好代码。你们两位支撑自己观点的原因都不太合适。最重要的还是代码的合理性，这部分错误code处理的逻辑本该属于谁，后续很可能出现的扩展是否方便支持，代码是否可以简洁易懂，是否会带来冗余代码。

tabris17

Nov 7, 2014

I打头的不应该都是interface么

tabris17

Nov 7, 2014

另外回答呵呵就好了

eric_zyh

Nov 7, 2014

看场景
1.如果 $uid < 10000 是一个常用的判断，实现成一个函数是很方便维护的。
2.如果只是在一两处地方有这个判断，就没有必要了。把只出现一、两次的都弄成函数，那维护起来反而不知道每个函数的含义。

如@tedeyang 所说的，这种代码考虑效率问题，那就太矫情了。。

kmvan

Nov 7, 2014

lz 这段代码是干啥的呢？是要判断用户是否登录？还是要验证登录信息是否正确？如果是判断登录，不就一个函数可以搞定吗？如果是验证登录信息，我觉得 wp 那种比这个更好理解。

zhouzm

Nov 7, 2014

抛开性能问题不谈，第二种写法，明显是对对象理解有问题，只是对第一段代码的形式上改写。

既然对象方法可能会有异常结果，那就不应该直接取值，应该是首先执行方法，由方法返回状态，如果成功，取值，不成功，则返回错误信息：

$login = new ILogin();
if !($login->doLogin()) {
return json_encode($login->getErrorl());
}
$uid = $login->getLoginUid();

反观第一段代码没有任何问题，由返回值来代表状态，改进的话只要把“array('ret'=>-1)”改为$login->getErrorl()就解决了错误信息封装的问题。

根本不是什么 OOP 的问题，是逻辑问题，楼主你好好思考一下。

aidevs

Nov 7, 2014

我觉得只要能发挥出OOP的好处，就可以用OOP

raincious

Nov 7, 2014

@chemzqm

这不是过渡封装。
https://gist.github.com/raincious/037f56d050ae92f01aa6

我觉得楼主唯一的问题是

if ($login->hasError()) {

这句定义的不明确，什么都是hasError。如果非要这样不如throw一个异常然后一个一个catch。

另外关于性能，那东西不是交给编译器优化的么？不要尝试过渡优化你的程序（微优化大部分情况下只是在浪费时间），应该尽量写出易于（让别人乐于）阅读的代码。

bombless

Nov 7, 2014

看什么产品什么公司吧，改动的比较猛的自己就会改一种写法，没有动力改其实也说明对可维护性的要求也不那么急迫

feinux

Nov 7, 2014

要我说，代码写出来是要用程序实现的。程序是给用户用的。用户是人。所以归根到底，他妈的你返回一句人看不懂的提示，老子用你的程序干蛋啊？除非是国企这种的，花大价钱搞来非用不可。

最讨厌一类程序员就是图自己省事儿，为难用户。还自己觉得自己很牛逼，用户是傻逼不懂。用户是不懂，用户可以不用啊？放着那么多好的程序不用，老子干吗委屈自己？

你把这段话复制过去，就说是我说的。

rwx

Nov 7, 2014

这是技术问题吗？这明明是人的问题
不是所有人都喜欢被别人无故改掉自己的代码，那是很直白的在告诉他：你写的真烂

不喷你喷谁。。

princecauchy

Nov 7, 2014

@jox object-oriented programming 面向对象编程。

latyas

Nov 7, 2014

原来的代码运行的好好的何必修改。

ren2881971

Nov 7, 2014

也不是ｏｏｐ啊　就是封装了下函数而已。
但是我觉得ＬＺ这种做法是对的！
告诉那个人万一 $uid < 10000 这个条件变了且不是应用中所有的代码都挨个检查下更改了？
封装成方法直接改方法内部就可以啦啊。

palxex

Nov 7, 2014

@jemyzhang 这个。。。招行信用卡的查询密码原来可以不设的么？我记得办卡时就设好了啊。（不是说交易密码啊，那个确实可以不设，而且我从来不设，绑定微信也没发现问题。）

pljhonglu

Nov 7, 2014

如果多处靠判断 uid 来判断正确性，那可以单独封装。否则还是觉得第一种比较直观。大项目为了可维护性必须要多封装。小工程本来模块间的耦合度就低，没必要过度封装。

另外，同意上面的说法，OOP的写法应该是抛异常

dreampuf

Nov 7, 2014

- 除了正确执行，代码就是用来维护的。谈维护有个标准，就是谁都能执行，大家品味一致
- 你能兼容他的hard code，他未必能兼容你的OOP
- 除非没人愿意接手维护了，或者有强力Leader牵头，不要干重构这类吃力不讨好的事儿。Martin 的书有点像心灵鸡汤，描绘了该是什么什么样。问题是design一旦脱离实际执行的人就变了味道，执行不好，培训不够，监督不全都会慢慢变坏。
- 没有数据，“可维护性”很有可能不可度量。比起明眼就能看出来的调用效率，自然被攻击。你需要想办法证明“这样写”的代码不可维护。

konakona

Nov 7, 2014

楼主应该是再没有了解业务环境的情况下妄自修改的。
如果说这是QQ登录，10000以前的不让用因为那是内部帐号，原来的代码将很好的表明这一点，而不需要翻来翻去甚至不用注释。

你改了以后反而费解。
并不是什么东西都要封装的。

barbery

Nov 7, 2014

大部分情况下，楼主这样写是对的。。。

zythum

Nov 7, 2014

他喵的这个和oop有半毛钱关系

coofly

Nov 7, 2014

$login = new ILogin();
$uid = 0;
if ($login->getLoginUid(&uid)) {
//use uid and other
} else {
return json_encode(array('ret'=>-1));
}

两个都有问题
感觉应该这么写吧？
让getLoginUid返回成功与否不就好了

不会php，可能有谬误

ruchee

Nov 7, 2014 via Android

第一种容易理解，也方便以后的维护，实在没必要去取错误记录绕弯子

railgun

Nov 7, 2014

具体情况具体分析咯，反正编程就是在可维护性和性能之前找平衡

hitsmaxft

Nov 7, 2014

我觉得吧, 为了方法而方法确实是浪费性能, 也谈不上什么oop, oop得解决具体场景才谈得上设计, 你这连问题都算不上, 只是个方法调用而已

判断一个登录失败, 调用了n个方法, 也没啥必要.

在登录失败的情况下, 直接给login加个属性

$login = new ILogin();

if (!$login->done) {
return ILogin::JSON_FAILED
}

ryd994

Nov 7, 2014 via Android

你可以搭个脚手架和他比比性能差多少。如果

ffffwh

Nov 8, 2014

OOP核心要义：多态。

herozzm

Nov 8, 2014

个人觉得，如果只是部分改造，赞成第一种写法，如果是全部重构，我赞成第二种写法

julyclyde

Nov 8, 2014

总感觉俩都挺怪的