欢迎您访问程序员文章站本站旨在为大家提供分享程序员计算机编程知识!
您现在的位置是: 首页  >  后端开发

如何改进这段代码

程序员文章站 2022-05-23 17:45:08
...
如下伪代码,
AService do2对P进行了操作,然后碰到的问题是PService里对P进行操作,如果调用了AService do2那么
由于不是对P最新的引用,save时会把AService do2的修改覆盖掉。

//Update 2015年05月22日17:24:28
@Ke_Wu 这不应该是逻辑问题,事实上,我作为后来的调用者没必要也不可能知道AService::do2里的具体实现,但现在碰到问题了,那么就是设计的问题了

class AService
{
    function do2(pid)
    {
        ...

        p = P.getById(pid);

        p.s = 'zz';

        p.save();

        ...
    }
}

class PService
{
    function do1(pid)
    {
        ...

        p = P.getById(pid);

        p.s = 'yy';

        AService.do2(pid);

        ...
        p.a = 'a';p.b = 'b';
        ...

        p.save();//p.s 仍旧是yy, zz被yy覆盖

        ...
    }
}

class CService
{
    function do4(cid)
    {
        ...
        c = C.getById(cid);
        pid = c.pid;
        AService.do2(pid);

        ...
    }
}

回复内容:

如下伪代码,
AService do2对P进行了操作,然后碰到的问题是PService里对P进行操作,如果调用了AService do2那么
由于不是对P最新的引用,save时会把AService do2的修改覆盖掉。

//Update 2015年05月22日17:24:28
@Ke_Wu 这不应该是逻辑问题,事实上,我作为后来的调用者没必要也不可能知道AService::do2里的具体实现,但现在碰到问题了,那么就是设计的问题了

class AService
{
    function do2(pid)
    {
        ...

        p = P.getById(pid);

        p.s = 'zz';

        p.save();

        ...
    }
}

class PService
{
    function do1(pid)
    {
        ...

        p = P.getById(pid);

        p.s = 'yy';

        AService.do2(pid);

        ...
        p.a = 'a';p.b = 'b';
        ...

        p.save();//p.s 仍旧是yy, zz被yy覆盖

        ...
    }
}

class CService
{
    function do4(cid)
    {
        ...
        c = C.getById(cid);
        pid = c.pid;
        AService.do2(pid);

        ...
    }
}

简化下来其实问题就是:

phpp1 = P.getById(pid);
p1.s = 'yy';
...
    p2 = P.getById(pid);
    p2.s = 'zz';
    p2.save();
...
p1.save();

保存了p2的修改(可能是存到数据库),并不意味着内存里的p1随之更新,除非你重新get一遍p1。
重构的目的是用来改善正确工作代码的风格和设计。
这段代码的问题是逻辑错误,对它而言谈重构还为时过早。

你的问题的本质,是两个“主语”(只是在你的案例中恰好都是service而已)的各自一个“行为”(do1 和 do2)含有了完全相同的一个“行动效果”(修改p.s的值)。
冲突不在于service,而在于行动效果冗余。
试想一下,换一个案例,其中只有一个主语,两个行为(do1 和 do2)都是它的,那么问题也是等价的。
两个行为有重叠的行动效果,实在太常见的了。
关键在于,你怎样界定,哪种重叠是满足需求的?哪种是错误、不合理的?

举一个满足需求的例子:
需求是:p是一个鼠标悬停的tips(界面组件)。先根据鼠标坐标,赋值p.top为一个值。随后,计算tips是否超出了窗口边缘。如果是,则计算tips的top的最大值(因为窗口大小可能会被改变,所以需要计算),然后赋值p.top为该最大值。p.left同理。
这是我做网页前端开发时遇到过的需求。

你的解决办法,大概可以解决你的那一个具体案例,但换成别的情况可能就又不对症了。
在我看来,关键在于,一个行为的源头(往往是事件)所导致一连串行动效果,其中要避免出现重叠;除非需求要求必要的重叠。
这“一连串”的“串法”,是设计上要想清楚的。你已经在朝这个方向努力了,只是关注点稍有偏离。
至于串的过程中的对象(主语/宾语)是不是service、是何种service,倒是没有关系。

我的解决办法如下,有什么缺点请指教:

Service应该分为2种:1,名词Service; 2, 行为Service
如:UserService 与 RegisterService

对于【名词Service】其里面每个method都必须返回相应的对象,如UserService下的upgrade(uid)就必须返回被升级后的user对象。

对于【行为Service】只对外暴露出一个execute(data),excute(data)必须返回行为成功与否的状态以及被施加这个行为的对象,如RegisterService下的excute(data)就必须返回注册成功与否,以及如果成功了它影响的对象。

通常我们约定对外只调用【行为Service】,再在【行为Service】里调用多个【名词Service】和其他【行为Service】,如在RegisterService::execute(data)里调用UserService::create(), UserService::markNewbee(uid),SendEmailService::execut()等;
【名词Service】中不允许调用【行为Service】。

所有Service的每个method的入参都可以是id或者对象实例,如upgrade()可以接受uid也可以接受user作为入参。

回到我的提问,可以这么写

class AService
{
    function get(aid_or_object)
    {
        if (aid_or_object instanceOf A) {
            return aid_or_object;
        }
        return A.getById(aid);
    }
}

class PService
{
    function get(pid_or_object)
    {
        if (pid_or_object instanceOf P) {
            return pid_or_object;
        }
        return P.getById(pid);
    }
}


class Do2Service
{
    function execute(aid_or_object, pid_or_object = null)
    {
        a = AService.get(aid_or_object);
        if (pid_or_object instanceOf P) {
            p = pid_or_object
        } else {
            p = PService.get(a.pid);
        }
        p.s = 'zz';
        p.save();
        a.save();
        return [:success, a, p];
    }
}

class Do3Service
{
    function execute(pid_or_object)
    {
        p = PService.get(pid_or_object);
        p.s = 'cc';
        p.save();
        return [:success, p];
    }
}

class Do1Service
{
    function execute(pid_or_object)
    {
        p = PService.get(pid_or_object);
        p.s = 'yy' if condition1        
        result, a, p = Do2Servce.execute(p.aid, p) if condition2
        result, p = Do3Servce.execute(p) if condition3

        p.a = 'a';
        p.b = 'b';

        p.save()

        return [:success, p, a];
    }
}

想要解决什么问题?