V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
johnsneakers
V2EX  ›  程序员

我写得 OOP 今天被喷了

  •  
  •   johnsneakers · 2014-11-07 10:57:22 +08:00 · 6657 次点击
    这是一个创建于 3450 天前的主题,其中的信息可能已经有所发展或是发生改变。
    事情是这样的,原来的代码是这样:

    $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, 但不知道这种情况改怎么反驳
    第 1 条附言  ·  2014-11-07 11:40:30 +08:00
    统一回复下:

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

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

    3.我一直觉得我的写法是对的。但对方掐我死穴,调用太多方法效率比起他那个稍显不足这一点我无法反驳。 我弱弱的说我的方便维护,好像不起啥作用。我只是想请教各位遇到这种情况怎么说服别人。。。。。。
    51 条回复    2014-11-08 18:28:05 +08:00
    chmlai
        1
    chmlai  
       2014-11-07 11:03:05 +08:00
    速度慢~哈哈哈
    pykwokcc
        2
    pykwokcc  
       2014-11-07 11:10:06 +08:00
    让我来写,我也会写成楼主的写法
    yxz00
        3
    yxz00  
       2014-11-07 11:10:06 +08:00
    我倒是觉得原来那个版本好些。只不过把那个magic num改成login的属性maxid就好了。
    yxz00
        4
    yxz00  
       2014-11-07 11:10:38 +08:00
    这个跟oop也没什么关系
    haiyang416
        5
    haiyang416  
       2014-11-07 11:11:17 +08:00 via Android
    从这几行代码里我没看出你拿 uid 做什么呀,第一个反倒很明白。
    我始终觉得不是为了面向对象而面向对象,不是所有东西都需要封装。
    这仅对你给的几行代码而言,团队项目的话一切按约定来,个人项目随便了。
    sivacohan
        6
    sivacohan  
       2014-11-07 11:14:43 +08:00
    我写成类似上面那个被喷了。
    咱俩换公司把。
    jemyzhang
        7
    jemyzhang  
       2014-11-07 11:16:03 +08:00
    国内很多都喜欢简单,而不考虑维护性.

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

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

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

    赞楼主~
    fengliu222
        8
    fengliu222  
       2014-11-07 11:17:04 +08:00
    无法评判这两段代码的好坏。
    你写的那段,$uid压根儿没用上。
    还有就是,把错误封装成一个方法hasError是否有必要,是要看错误码的多少,如果错误情况比较多,那么集中处理判断确实挺好的。如果只有一个,就有点设计过度了。。
    PrideChung
        9
    PrideChung  
       2014-11-07 11:20:19 +08:00   ❤️ 1
    如果是我就一定会用看小狗的眼神看着他说:“要不要给你介绍几本程序设计的入门书?”
    patr0nus
        10
    patr0nus  
       2014-11-07 11:20:45 +08:00
    搭配LZ头像看贴风味更佳;)
    zakokun
        11
    zakokun  
       2014-11-07 11:21:38 +08:00
    我倒是觉得原来那个语义简介好理解....不过楼主的话配合你的头像很有喜感
    hslx111
        12
    hslx111  
       2014-11-07 11:24:40 +08:00
    这个代码没有上下文,很难判断谁对谁错
    cxshun
        13
    cxshun  
       2014-11-07 11:27:49 +08:00
    就一行调用还真没必要了。但以速度慢来反驳,只能说哈哈了。
    leiz
        14
    leiz  
       2014-11-07 11:28:32 +08:00
    如果< 10000和 ret -1那个用的地方很多,从维护的角度看,你的做法是对的。不然等以后维护的时候,接手的人哭死。
    如果只是一两处,看喜好。我个人会偏向你的写法,否则以后要调整这些错误体系的话,折腾死。
    chemzqm
        15
    chemzqm  
       2014-11-07 11:35:55 +08:00
    OO的思路更适合功能复杂的产品,简单的调用更适合糙快猛的项目,如果需求多变项目,真没必要过度封装,只会增加所有人的理解成本。
    fising
        16
    fising  
       2014-11-07 11:38:19 +08:00
    真是啥都能喷。。。
    bingu
        17
    bingu  
       2014-11-07 11:44:54 +08:00
    支持楼主派和反对楼主派梅花间竹地出现了。
    levn
        18
    levn  
       2014-11-07 11:46:00 +08:00   ❤️ 1
    快,把json_encode也封起来
    curiousjude
        19
    curiousjude  
       2014-11-07 12:02:49 +08:00
    @fengliu222 楼主这只是一个代码片段而言,都hasError了,uid当然是没有啦。另外,错误码的多少不一定是一成不变的,系统复杂后,可能的错误情况也会增多。
    tedeyang
        20
    tedeyang  
       2014-11-07 12:10:56 +08:00   ❤️ 1
    1,用uid兼做错误代码也真是醉了。这种代码已经不是维护性差的程度,是二义性错误。
    2,性能问题有个经典答案:如无必要,不需优化。优化了方法调用的几个纳秒对以ms计的HTTP网络传输有什么意义?要快请用opcode,用java,用C。
    3,你的代码也很烂。model、validation、error不应该这么耦合。如果遇到允许为false的字段,你这个机制就出问题了。
    这么喜欢OOP,还不转投java阵营?Spring、Annotation、Bean的设计在解决你这个问题上甩了几条街。。。
    jox
        21
    jox  
       2014-11-07 12:16:43 +08:00
    OOP是啥谁来告诉我
    RemRain
        22
    RemRain  
       2014-11-07 12:41:19 +08:00   ❤️ 1
    hasError 不是面向过程的方法么,OOP 的话,抛个异常吧
    johnsneakers
        23
    johnsneakers  
    OP
       2014-11-07 12:47:26 +08:00
    @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
        24
    loryyang  
       2014-11-07 12:57:34 +08:00
    过早的性能优化是罪恶的源泉,特别是为了性能而去牺牲系统架构优雅性,增加模块耦合,降低代码可读性、易维护性等。当然,如果没有后面这些损失,你当然应该写更高效的代码。

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

    关于你这个代码,我觉得原来的代码也还可以,oop不是解决问题的唯一方式,不用oop也一样可以写出好代码。你们两位支撑自己观点的原因都不太合适。最重要的还是代码的合理性,这部分错误code处理的逻辑本该属于谁,后续很可能出现的扩展是否方便支持,代码是否可以简洁易懂,是否会带来冗余代码。
    tabris17
        25
    tabris17  
       2014-11-07 12:59:27 +08:00
    I打头的不应该都是interface么
    tabris17
        26
    tabris17  
       2014-11-07 13:01:53 +08:00
    另外回答呵呵就好了
    eric_zyh
        27
    eric_zyh  
       2014-11-07 13:04:21 +08:00
    看场景
    1.如果 $uid < 10000 是一个常用的判断,实现成一个函数是很方便维护的。
    2.如果只是在一两处地方有这个判断,就没有必要了。把只出现一、两次的都弄成函数,那维护起来反而不知道每个函数的含义。

    如@tedeyang 所说的,这种代码考虑效率问题,那就太矫情了。。
    kmvan
        28
    kmvan  
       2014-11-07 13:04:58 +08:00
    lz 这段代码是干啥的呢?是要判断用户是否登录?还是要验证登录信息是否正确?如果是判断登录,不就一个函数可以搞定吗?如果是验证登录信息,我觉得 wp 那种比这个更好理解。
    zhouzm
        29
    zhouzm  
       2014-11-07 13:37:51 +08:00   ❤️ 1
    抛开性能问题不谈,第二种写法,明显是对对象理解有问题,只是对第一段代码的形式上改写。

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

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

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

    根本不是什么 OOP 的问题,是逻辑问题,楼主你好好思考一下。
    dbfox
        30
    dbfox  
       2014-11-07 13:47:29 +08:00
    我觉得只要能发挥出OOP的好处,就可以用OOP
    raincious
        31
    raincious  
       2014-11-07 13:58:06 +08:00   ❤️ 1
    @chemzqm

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

    我觉得楼主唯一的问题是

    if ($login->hasError()) {

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

    另外关于性能,那东西不是交给编译器优化的么?不要尝试过渡优化你的程序(微优化大部分情况下只是在浪费时间),应该尽量写出易于(让别人乐于)阅读的代码。
    bombless
        32
    bombless  
       2014-11-07 14:52:27 +08:00
    看什么产品什么公司吧,改动的比较猛的自己就会改一种写法,没有动力改其实也说明对可维护性的要求也不那么急迫
    feinux
        33
    feinux  
       2014-11-07 15:27:24 +08:00
    要我说,代码写出来是要用程序实现的。程序是给用户用的。用户是人。所以归根到底,他妈的你返回一句人看不懂的提示,老子用你的程序干蛋啊?除非是国企这种的,花大价钱搞来非用不可。

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

    你把这段话复制过去,就说是我说的。
    rwx
        34
    rwx  
       2014-11-07 15:55:36 +08:00
    这是技术问题吗?这明明是人的问题
    不是所有人都喜欢被别人无故改掉自己的代码,那是很直白的在告诉他:你写的真烂

    不喷你喷谁。。
    princecauchy
        35
    princecauchy  
       2014-11-07 16:06:01 +08:00
    @jox object-oriented programming 面向对象编程。
    latyas
        36
    latyas  
       2014-11-07 16:13:20 +08:00
    原来的代码运行的好好的何必修改。
    ren2881971
        37
    ren2881971  
       2014-11-07 16:18:33 +08:00
    也不是oop啊 就是封装了下函数而已。
    但是我觉得LZ这种做法是对的!
    告诉那个人 万一 $uid < 10000 这个条件变了 且不是应用中所有的代码都挨个检查下更改了?
    封装成方法 直接改方法内部就可以啦啊。
    palxex
        38
    palxex  
       2014-11-07 16:52:52 +08:00
    @jemyzhang 这个。。。招行信用卡的查询密码原来可以不设的么?我记得办卡时就设好了啊。(不是说交易密码啊,那个确实可以不设,而且我从来不设,绑定微信也没发现问题。)
    pljhonglu
        39
    pljhonglu  
       2014-11-07 16:54:38 +08:00   ❤️ 1
    如果多处靠判断 uid 来判断正确性,那可以单独封装。否则还是觉得第一种比较直观。大项目为了可维护性必须要多封装。小工程本来模块间的耦合度就低,没必要过度封装。

    另外,同意上面的说法,OOP的写法应该是抛异常
    dreampuf
        40
    dreampuf  
       2014-11-07 17:02:24 +08:00   ❤️ 1
    - 除了正确执行,代码就是用来维护的。谈维护有个标准,就是谁都能执行,大家品味一致
    - 你能兼容他的hard code,他未必能兼容你的OOP
    - 除非没人愿意接手维护了,或者有强力Leader牵头,不要干重构这类吃力不讨好的事儿。Martin 的书有点像心灵鸡汤,描绘了该是什么什么样。问题是design一旦脱离实际执行的人就变了味道,执行不好,培训不够,监督不全都会慢慢变坏。
    - 没有数据,“可维护性”很有可能不可度量。比起明眼就能看出来的调用效率,自然被攻击。你需要想办法证明“这样写”的代码不可维护。
    konakona
        41
    konakona  
       2014-11-07 17:07:45 +08:00
    楼主应该是再没有了解业务环境的情况下妄自修改的。
    如果说这是QQ登录,10000以前的不让用因为那是内部帐号,原来的代码将很好的表明这一点,而不需要翻来翻去甚至不用注释。

    你改了以后反而费解。
    并不是什么东西都要封装的。
    barbery
        42
    barbery  
       2014-11-07 17:13:50 +08:00
    大部分情况下,楼主这样写是对的。。。
    zythum
        43
    zythum  
       2014-11-07 17:40:14 +08:00
    他喵的这个和oop有半毛钱关系
    coofly
        44
    coofly  
       2014-11-07 17:47:32 +08:00
    $login = new ILogin();
    $uid = 0;
    if ($login->getLoginUid(&uid)) {
    //use uid and other
    } else {
    return json_encode(array('ret'=>-1));
    }

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

    不会php,可能有谬误
    ruchee
        45
    ruchee  
       2014-11-07 18:24:00 +08:00 via Android
    第一种容易理解,也方便以后的维护,实在没必要去取错误记录绕弯子
    railgun
        46
    railgun  
       2014-11-07 18:31:59 +08:00
    具体情况具体分析咯,反正编程就是在可维护性和性能之前找平衡
    hitsmaxft
        47
    hitsmaxft  
       2014-11-07 21:40:58 +08:00
    我觉得吧, 为了方法而方法确实是浪费性能, 也谈不上什么oop, oop得解决具体场景才谈得上设计, 你这连问题都算不上, 只是个方法调用而已

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

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


    $login = new ILogin();

    if (!$login->done) {
    return ILogin::JSON_FAILED
    }
    ryd994
        48
    ryd994  
       2014-11-07 22:03:35 +08:00 via Android
    你可以搭个脚手架和他比比性能差多少。如果
    ffffwh
        49
    ffffwh  
       2014-11-08 00:40:52 +08:00
    OOP核心要义:多态。
    herozzm
        50
    herozzm  
       2014-11-08 01:36:45 +08:00
    个人觉得,如果只是部分改造,赞成第一种写法,如果是全部重构,我赞成第二种写法
    julyclyde
        51
    julyclyde  
       2014-11-08 18:28:05 +08:00
    总感觉俩都挺怪的
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   我们的愿景   ·   实用小工具   ·   1133 人在线   最高记录 6543   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 29ms · UTC 18:29 · PVG 02:29 · LAX 11:29 · JFK 14:29
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.