首页新闻找找看学习计划

代码重构的问题

0
悬赏园豆:30 [已解决问题] 解决于 2018-06-28 10:26

感觉方法的结构都差不多,但就是没有办法重构。
例如下边这几个方法

public IUnivBaseResult<IEnumerable<Message>> GetAllMessagePageNew(long projectId, int pid, int start, int pageSize)
        {
            try
            {
                var cacheKey = $"{nameof(GetAllMessagePageNew)}_{projectId}_ {pid}_ {start}_{pageSize}";
                var result = _cachingProvider.Get<IEnumerable<Message>>(cacheKey);
                if (!result.HasValue)
                {
                    _cachingProvider.Set(cacheKey,
                           _repository.GetQuery(e => e.ProjectId == projectId
                                             && e.ReceiverPid == pid
                                             && e.Status != MessageStatus.Delete)
                                 .OrderBy(e => e.Status)
                                 .Skip(start)
                                 .Take(pageSize)
                                 .ToList(),
                           TimeSpan.FromMinutes(1));
                    result = _cachingProvider.Get<IEnumerable<Message>>(cacheKey);
                }
                return UnivBaseResult<IEnumerable<Message>>.Success(result.Value);
            }
            catch (Exception e)
            {
                _log.LogError($"{e}");
                return UnivBaseResult<IEnumerable<Message>>.Failed(-1, e.Message);
            }
        }

类似的另一个方法

 public IUnivBaseResult<IEnumerable<Message>> GetUserAllMessageNew(long userId, int pid, long projectId)
        {
            try
            {
                var cacheKey = $"{nameof(GetUserAllMessageNew)}_{userId}_{pid}_{projectId}";
                var result = _cachingProvider.Get<IEnumerable<Message>>(cacheKey);
                if (!result.HasValue)
                {
                    _cachingProvider.Set(cacheKey,
                        _repository.GetQuery(e => e.ProjectId == projectId
                                            && e.Pid == pid
                                            && e.UserId == userId
                                            && e.Status != MessageStatus.Delete)
                                  .OrderBy(e => e.Status)
                                  .ToList()
                        , TimeSpan.FromMinutes(1));
                }
                return UnivBaseResult<IEnumerable<Message>>.Success(result.Value);
            }
            catch (Exception e)
            {
                _log.LogError(e.Message);
                return UnivBaseResult<IEnumerable<Message>>.Failed(-1, e.Message);
            }
        }

是否能提供重构的思路?或者应该从哪个方面努力。

Bluto的主页 Bluto | 菜鸟二级 | 园豆:317
提问于:2018-06-22 15:05
< >
分享
最佳答案
0

感谢大家的帮助,最终重构掉了很多代码。
最终的结果

// try catch 放在了全局异常处理中去了。
//全局异常处理
  public override void OnActionExecuted(ActionExecutedContext context)
        {
            dynamic controller= this;
            ILogger log = controller._log;
            log.LogError($"{context.Exception}");
        }
//缓存处理
    private T GetFromCache<T1, T2, T>(string cacheKey, T1 key, T2 pid, Func<T1, T2, T> func)
        {
            var result = _cachingProvider.Get<T>(cacheKey);
            if (!result.HasValue)
            {
                _cachingProvider.Set(cacheKey, func.Invoke(key, pid), TimeSpan.FromMinutes(1));
                result = _cachingProvider.Get<T>(cacheKey);
            }
            return result.Value;
        }
//改造结果
        public IUnivBaseResult<IEnumerable<Message>> GetUserAllMessageNew(long userId, int pid, long projectId)
        {

            var cacheKey = $"{nameof(GetUserAllMessageNew)}_{userId}_{pid}_{projectId}";
            var result = GetFromCache(cacheKey,userid,pid,projectId,someMethod);
            return Success(result.Value);

        }

很是清爽,省了很多代码。

Bluto | 菜鸟二级 |园豆:317 | 2018-06-28 10:23
其他回答(5)
0

 总共才几行.你想怎么重构..吧换行符删掉?

重构基本上就是复用代码提取.

就是去除重复代码.

吴瑞祥 | 园豆:28675 (高人七级) | 2018-06-22 15:15

这两个方法的结构几乎就是一样的。感觉是不是重复能更简单一点呢?

支持(0) 反对(0) Bluto | 园豆:317 (菜鸟二级) | 2018-06-22 15:17
0

这确实是可以重构的:

首先这是一个获取缓存的函数是吧,那这个函数写成这样可以好点

public T getCache(string CacheKey, Func<T> GetValueFunc)
{
    if(key exsit)
       return cacheValue;
    else
       var r= GetValueFunc.Invoke;
       setCache(key, r);
       return r;
}

伪代码而已,不必当真。

 

这样一来,你的缓存类就可以单独写,而无需和其他业务类写在一起。

Func<T> 用于获取从数据库或其他持久化设施或网络中获取最新的数据。

收获园豆:10
爱编程的大叔 | 园豆:29788 (高人七级) | 2018-06-22 15:35

理解了,多谢!

支持(0) 反对(0) Bluto | 园豆:317 (菜鸟二级) | 2018-06-22 16:24
0

你想把两个函数变成一个函数吗?

Shendu.CC | 园豆:1811 (小虾三级) | 2018-06-22 16:22

不是的这个两个方法相似度太高了,能不能更抽象一点

支持(0) 反对(0) Bluto | 园豆:317 (菜鸟二级) | 2018-06-22 16:23

感觉是在写重复代码。

支持(0) 反对(0) Bluto | 园豆:317 (菜鸟二级) | 2018-06-22 16:24
0

1.try catch不需要,既然都有放到外部过程去处理,——反而需要做的可能是更细的throw;

2.如果也都是cache,这个也放到外部过程,具体实现只放 参数转换 实现函数或者getter,如果查询都是由GetQuery发生调用可以直接扩展一个该函数——其中包含cache过程。

这样你以上代码就少了几行。

eg.

        protected override dynamic Create(HttpListenerContext context, Account entity)
        {
            var orgAccount = SessionAuth.GetOrgAccount(context.GetSession().DbName);
            if (Table.Count() > orgAccount.QuantityUser) throw new Warn($"你的企业帐号资源限制{orgAccount.QuantityUser}个");
            if (string.IsNullOrEmpty(entity.User)) throw new Warn("未填写帐号");
            if (entity.Role == byte.MinValue) throw new Warn("未指定职能");
            if ((entity.Role & Role.普工)== Role.普工 && entity.Ability == byte.MinValue) throw new Warn("未设定普工限制");
            if (string.IsNullOrEmpty(entity.Pwd)) throw new Warn("未填写密码");
            if (string.IsNullOrEmpty(entity.Tel)) throw new Warn("未填写电话");
            if(Table.Any(t=>t.User == entity.User)) throw new Warn("已存在该帐号");
            return base.Create(context, entity);
        }

总而言之,但凡存在ctrl c v操作的,大多数情况你都应该考虑放入过程去处理。——这也是所谓框架的意义。

收获园豆:10
花飘水流兮 | 园豆:10297 (专家六级) | 2018-06-23 01:26

多谢提醒try catch已经去掉,放在了了全局异常处理中了。

支持(0) 反对(0) Bluto | 园豆:317 (菜鸟二级) | 2018-06-28 10:15
0

在这园子里找找 “规约模式” 或者 “Specification Pattern”这个关键词

收获园豆:10
西漠以西 | 园豆:1670 (小虾三级) | 2018-06-23 08:49
清除回答草稿
   您需要登录以后才能回答,未注册用户请先注册