- 主题:一个封得巨烂但被大量调用的函数，有啥妙招重构不？
两个字：巨烂
1、把成功字符串和拼凑的错误原因字符串放在同一个返回值里；
2、有两三百处调用这个函数，到处都要改调用代码；
3、无缓存层（只缓存了etag，没缓存返回的成功串）；
4、http 304时，调用者还要判断一下字符串是不是“Content not modified”，此时要使用调用者自己缓存的上次的成功字符串;
5、http 404应该是给服务端擦屁股的（估计服务端在负载大时可能返回404，才那么写）;
骨架大概长这样。貌似AI也没有特别令人满意的招数，难以避免修改那几百处调用的代码。
std::string HttpPost(const std::string &url, const std::string &body1, const bool use_etag) {
std::stringstream result;
try {
std::string err;
auto reqBody = EncryptMessage(body1, err, kDefaultCryptProtocol);
if (!err.empty()) {
CLOG4_DEBUG("[HttpPost] encrypt message failed due to " << err);
return "###Error### encrypt message failed";
}
...
long responseCode = curlpp::infos::ResponseCode::get(request.GetCurl());
switch (responseCode) {
case 200:
break;
case 304:
case 404:
return std::string("Content not modified");
default:
return std::string("###Error### Cloud Error Code: ") + std::to_string(responseCode);
}
} catch (curlpp::LogicError &e) {
CLOG4_DEBUG("[HttpPost] LogicError " << e.what());
return std::string("###Error###") + e.what();
} catch (curlpp::RuntimeError &e) {
CLOG4_DEBUG("[HttpPost] RuntimeError " << e.what());
return std::string("###Error###") + e.what();
}
...
std::string err;
auto plaintext = DecryptMessage(result.view(), err);
if (!err.empty()) {
CLOG4_DEBUG("[HttpPost] decrypt message failed due to " << err);
return "###Error### decrypt message failed";
}
return plaintext;
}
--
FROM 111.199.147.*
丢给 AI 呗，看看 AI 有啥优化的妙招。
【 在 z16166 的大作中提到: 】
: 两个字：巨烂
: 1、把成功字符串和拼凑的错误原因字符串放在同一个返回值里；
: 2、有两三百处调用这个函数，到处都要改调用代码；
: ...................
--
FROM 124.64.23.*
返回一个垫片类
把字符串拼接放在operator string()里
用operator bool()判断是否成功
【 在 z16166 的大作中提到: 】
: 两个字：巨烂
: 1、把成功字符串和拼凑的错误原因字符串放在同一个返回值里；
: 2、有两三百处调用这个函数，到处都要改调用代码；
: ...................
--
FROM 125.33.196.*
这个返回的字符串其实包含了跟调用者约定的协议。要改，只能跟调用者一起改。
这个函数的问题主要是把真正的返回值跟错误信息编码在一起了。
用个tuple,或者写个专门包装返回信息的类
template<class T>
class ReturnInfo
{
public :
T value;
int ret_code;
string ret_message;
}
原来的函数返回ReturnInfo<string> ，这样就很清晰。
但是必然要改调用者的代码。
【 在 z16166 的大作中提到: 】
: 两个字：巨烂
: 1、把成功字符串和拼凑的错误原因字符串放在同一个返回值里；
: 2、有两三百处调用这个函数，到处都要改调用代码；
: ...................
--
FROM 223.72.62.*
或者直接抛出异常，
在调用者更高的层级拦截处理。这样可以跳过调用函数里面的错误处理逻辑。
【 在 z16166 的大作中提到: 】
: 两个字：巨烂
: 1、把成功字符串和拼凑的错误原因字符串放在同一个返回值里；
: 2、有两三百处调用这个函数，到处都要改调用代码；
: ...................
--
FROM 223.72.62.*
关键看caller拿到返回值怎么用吧，都不知道caller怎么用的，没法改啊
--
FROM 115.192.110.*
楼主说了啊，调用者拿到返回值，要一个一个if来判断里面是什么。
按照特定的字符串来判定是什么错误。
找不到预定的错误，就是正常的返回值。
之前应该约定了正常返回值不会出现约定的错误信息标志。
【 在 ziqin 的大作中提到: 】
: 关键看caller拿到返回值怎么用吧，都不知道caller怎么用的，没法改啊
--
FROM 223.72.62.*
其实我觉得这个设计没啥问题，std::optional不也是这个设计么? 自己在类前面加一个状态字段。
--
FROM 115.192.110.*
重构的目的是什么？让代码可读性变强还是增加性能？如果是纯业务逻辑，没什么重构的必要
--
FROM 115.192.110.*
我不想在以后的新功能里再调用这个玩意儿了，哈哈
不改变已有调用者代码的办法，就是2楼dilemma说的办法，
针对老的调用者表现出接口语义不变，但改成返回一个带operator const std::string &()操作符重载的struct。新的调用者可以用这个struct里导出的新接口。
AI给的也是这个办法。
老调用者拿到混合的std::string后，
1、如果带###Error###，就认为失败，并记录失败log到文件。
2、如果是Content not modified，就需要从自己搞的缓存里拿出上次成功的串来使用。
3、否则就是成功，缓存到自己的某个地方，并使用。
其中，2不能简单和1一样以失败了之，因为有两个url接口的返回值要联合起来使用的情况，
比如：
url1返回成功（有新的策略或者配置数据），而url2返回Content not modified（策略或者配置数据没变）,
然后url1和url2的响应数据要做一个关联处理（实际就是把服务端的关联计算的开销给移到客户端了）。
这种情况下url2不能把Content not modified当作失败来处理，否则会导致url1的更新也等于是失败了。
另外，我打算把成功的结果统一在HttpPost()的内部进行缓存，而无需每个调用者自己缓存。
只缓存带etag的请求。
这个缓存在多线程下是存在race从而导致miss的，比如：
线程A先请求url1，拿到成功值，
而线程B以同样的参数后请求url1，拿到的是Content not modified，但此时线程A并不一定就把成功值放入缓存了，就产生cache miss。这种先就当作失败处理好了。
如果我放弃对现有这个HttpPost()的重构，调用这个老接口的代码也不动，
而另起炉灶搞一个HttpPost2(), 只给新功能用，也会因为上面说的这个统一缓存，而需要在老接口HttpPost()里对缓存进行CRUD。
【 在 ziqin 的大作中提到: 】
: 重构的目的是什么？让代码可读性变强还是增加性能？如果是纯业务逻辑，没什么重构的必要
--
FROM 111.199.147.*