代码重构的问题
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);
}
}是否能提供重构的思路？或者应该从哪个方面努力。
最佳答案
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);
}很是清爽，省了很多代码。
其他回答(5)
0
总共才几行.你想怎么重构..吧换行符删掉?
重构基本上就是复用代码提取.
就是去除重复代码.
这两个方法的结构几乎就是一样的。感觉是不是重复能更简单一点呢？
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
理解了，多谢！
0
你想把两个函数变成一个函数吗？
不是的这个两个方法相似度太高了，能不能更抽象一点
感觉是在写重复代码。
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
多谢提醒try catch已经去掉，放在了了全局异常处理中了。
0
在这园子里找找 “规约模式” 或者 “Specification Pattern”这个关键词
收获园豆：10
