避免两个函数之间的代码重复,但代码中间有一个 returns
Avoid code duplication between two functions, but one returns in the middle of the code
假设我在 class:
中有两个函数
vector<int> getAllPossibleNumbers() {
vector<int> nums;
for (int i=0; i < MAX; i++)
if (isAPossibleNumber(i))
nums.push_back(i);
return nums;
}
bool hasAtLeastOnePossibleNumber() {
for (int i=0; i < MAX; ++i)
if (isAPossibleNumber(i))
return true;
return false;
}
我可以将第二个函数重写为
bool hasAtLeastOnePossibleNumber() {
return getAllPossibleNumbers().size() > 0;
}
但很明显,尤其是在 MAX 数非常高的情况下,第一个效率更高。
那么,我怎样才能避免这种代码重复呢?
我更喜欢一个通用的解决方案,它可以处理更复杂的函数,并且可以用 C++11 编译。
我不会费心去重构代码,因为重复并不是那么激烈。这两个函数确实做了不同的事情。但是,如果您编写的函数只有 returns 个 int
:
const int MAX = 100;
bool isAPossibleNumber(int i){ return true;}
int getNextPossibleNumber(int start=0,int stop = MAX) {
for (int i=0; i < stop; i++) {
if (isAPossibleNumber(i)) return i;
}
return stop;
}
那么其中一个函数就简单短小
bool hasAtLeastOnePossibleNumber() {
return getNextPossibleNumber(0,MAX) != MAX;
}
而另一个变得更冗长
std::vector<int> getAllPossibleNumbers() {
std::vector<int> nums;
int start = 0;
while (start < MAX) {
auto i = getNextPossibleNumber(start);
if (i != MAX) nums.push_back(i);
start = i+1;
}
return nums;
}
请注意,总的来说这是更多的代码,所以这是一个权衡。如果您在其他地方也不需要 getNextPossibleNumber
,我可能会按原样保留您的代码。
另请注意,虽然这会稍微减少代码重复,但不会减少您需要完成的工作量,例如:
if (hasPossibleNumber()) {
auto vec = getAllPossibleNumbers();
}
在最坏的情况下,唯一可能的数字是 MAX-1
,并且两个函数都将迭代整个范围。与其让两个函数都完成工作,不如缓存结果并在再次调用其中一个函数时重用它。
一个选择是将大部分逻辑移动到第三个函数中,并回调提供数字的外部函数。回调的结果可以用来判断是否继续循环:
template <typename Callback>
void checkNumbers(Callback callback)
{
for (int i=0; i < MAX; i++)
{
if (isAPossibleNumber(i))
{
if (!callback(i))
{
break;
}
}
}
}
std::vector<int> getAllPossibleNumbers() {
std::vector<int> nums;
checkNumbers([&](int i){ nums.push_back(i); return true; });
return nums;
}
bool hasAtLeastOnePossibleNumber() {
bool result = false;
checkNumbers([&](int i){ result = true; return false; });
return result;
}
多余的函数应该被现代编译器优化掉。
您实际上没有任何代码重复。它看起来就像你做的,因为你已经使用基于索引的 for 循环来实现不同的算法。如果你到处都写这样的循环,它看起来就像你在重复代码。
你应该做的是使用合适的算法
vector<int> getAllPossibleNumbers()
{
vector<int> nums;
std::ranges::copy_if(std::views::iota(0, MAX),
std::back_inserter(nums),
isAPossibleNumber);
return nums;
}
bool hasAtLeastOnePossibleNumber()
{
return std::ranges::any_of(std::views::iota(0, MAX),
isAPossibleNumber);
}
现在真的没有什么可以去重的了。
*这是 C++20 代码,但您可以在旧版本中做几乎相同的事情。
假设我在 class:
中有两个函数vector<int> getAllPossibleNumbers() {
vector<int> nums;
for (int i=0; i < MAX; i++)
if (isAPossibleNumber(i))
nums.push_back(i);
return nums;
}
bool hasAtLeastOnePossibleNumber() {
for (int i=0; i < MAX; ++i)
if (isAPossibleNumber(i))
return true;
return false;
}
我可以将第二个函数重写为
bool hasAtLeastOnePossibleNumber() {
return getAllPossibleNumbers().size() > 0;
}
但很明显,尤其是在 MAX 数非常高的情况下,第一个效率更高。
那么,我怎样才能避免这种代码重复呢?
我更喜欢一个通用的解决方案,它可以处理更复杂的函数,并且可以用 C++11 编译。
我不会费心去重构代码,因为重复并不是那么激烈。这两个函数确实做了不同的事情。但是,如果您编写的函数只有 returns 个 int
:
const int MAX = 100;
bool isAPossibleNumber(int i){ return true;}
int getNextPossibleNumber(int start=0,int stop = MAX) {
for (int i=0; i < stop; i++) {
if (isAPossibleNumber(i)) return i;
}
return stop;
}
那么其中一个函数就简单短小
bool hasAtLeastOnePossibleNumber() {
return getNextPossibleNumber(0,MAX) != MAX;
}
而另一个变得更冗长
std::vector<int> getAllPossibleNumbers() {
std::vector<int> nums;
int start = 0;
while (start < MAX) {
auto i = getNextPossibleNumber(start);
if (i != MAX) nums.push_back(i);
start = i+1;
}
return nums;
}
请注意,总的来说这是更多的代码,所以这是一个权衡。如果您在其他地方也不需要 getNextPossibleNumber
,我可能会按原样保留您的代码。
另请注意,虽然这会稍微减少代码重复,但不会减少您需要完成的工作量,例如:
if (hasPossibleNumber()) {
auto vec = getAllPossibleNumbers();
}
在最坏的情况下,唯一可能的数字是 MAX-1
,并且两个函数都将迭代整个范围。与其让两个函数都完成工作,不如缓存结果并在再次调用其中一个函数时重用它。
一个选择是将大部分逻辑移动到第三个函数中,并回调提供数字的外部函数。回调的结果可以用来判断是否继续循环:
template <typename Callback>
void checkNumbers(Callback callback)
{
for (int i=0; i < MAX; i++)
{
if (isAPossibleNumber(i))
{
if (!callback(i))
{
break;
}
}
}
}
std::vector<int> getAllPossibleNumbers() {
std::vector<int> nums;
checkNumbers([&](int i){ nums.push_back(i); return true; });
return nums;
}
bool hasAtLeastOnePossibleNumber() {
bool result = false;
checkNumbers([&](int i){ result = true; return false; });
return result;
}
多余的函数应该被现代编译器优化掉。
您实际上没有任何代码重复。它看起来就像你做的,因为你已经使用基于索引的 for 循环来实现不同的算法。如果你到处都写这样的循环,它看起来就像你在重复代码。
你应该做的是使用合适的算法
vector<int> getAllPossibleNumbers()
{
vector<int> nums;
std::ranges::copy_if(std::views::iota(0, MAX),
std::back_inserter(nums),
isAPossibleNumber);
return nums;
}
bool hasAtLeastOnePossibleNumber()
{
return std::ranges::any_of(std::views::iota(0, MAX),
isAPossibleNumber);
}
现在真的没有什么可以去重的了。
*这是 C++20 代码,但您可以在旧版本中做几乎相同的事情。