重构开关或 if/else 语句?

Refactoring switch or if/else statement?

本文关键字：else 语句 if 开关 重构      更新时间：2023-10-16

我正在做一个学校项目，从老师那里得到了一些反馈。他说在我的代码中有一些不好的做法，他说开关案例可以用多态方法代替。只是我不知道我该怎么做。

我的代码正在接收来自CAN总线的消息。这些消息来自不同的设备，我检查它们来自哪个设备的消息。如果有新设备，我会创建一个对象并解析消息并存储信息。 对于每条消息，此系统几乎相同。

这是我的代码。

void Application::PollWhisperConnectBus()
{
HAL_GPIO_TogglePin(PORT_LED1, PIN_LED1);
whisper_connect_id_ = hcan2.pRxMsg->StdId;
if (whisper_connect_id_ >= 0x580 && whisper_connect_id_ <= 0x58F)
{
WIBDevice();
}
if (whisper_connect_id_ >= 0x590 && whisper_connect_id_ <= 0x59F)
{
BMSSDevice();
}
if (whisper_connect_id_ >= 0x5B0 && whisper_connect_id_ <= 0x5BF)
{
DCPowerCubeDevice();
}
if (whisper_connect_id_ >= 0x5C0 && whisper_connect_id_ <= 0x5CF)
{
ACPowerCubeDevice();
}
if (whisper_connect_id_ >= 0x700 && whisper_connect_id_ <= 0x70F)
{
WIBHeartBeatDevice();
}
}

这是检查是否存在类对象的函数之一，如果有，则解析消息。

void Application::DCPowerCubeDevice()
{
bool found_device = false;
int device = (hcan2.pRxMsg->StdId & 0x0F) + device_instance_offset_;
WhisperConnectDevice* whisper_connect_device;
for(unsigned int i = 0; i < whisper_connect_device_list_.size(); ++i)
{
if ((whisper_connect_device = whisper_connect_device_list_.at(i)) != NULL &&
whisper_connect_device->GetClassName() == "DCPowerCube")
{
DCPowerCube* dc_powercube = dynamic_cast<DCPowerCube*>(whisper_connect_device);
if (dc_powercube != NULL)
{
if (dc_powercube->GetDevice() == device)
{
dc_powercube->ParseCanMessage(&hcan2);
found_device = true;
break;
}
}
}
}
if (!found_device)
{
WhisperConnectDevice* dc_powercube;
if ((dc_powercube = new DCPowerCube) != NULL)
{
dc_powercube->SetDevice(device);
int n2k_address = nmea2000_.FindFirstFreeCanId(n2k_address_, device_list_);
if (n2k_address != 0xFFFF)
{
dc_powercube->SetSrcCanId(n2k_address);
dc_powercube->SetDeviceInstanceOffset(device_instance_offset_);
dc_powercube->SetDeviceInstance(0x30 + device);
dc_powercube->AddressClaim(nmea2000_);
dc_powercube->SendPGN126996(nmea2000_);
dc_powercube->SendPGN126998(nmea2000_, "DCPowerCube", "", "");
device_list_.at(n2k_address) = 0x01;
}
DCPowerCube* dc_powercube2 = dynamic_cast<DCPowerCube*>(dc_powercube);
if (dc_powercube2 != NULL)
{
dc_powercube2->SetCurrentLimit(16);
}
AddToWPCDeviceList(dc_powercube);
}
}
}
void DCPowerCube::ParseCanMessage(CAN_HandleTypeDef *can_handle)
{
if (can_handle != NULL)
{
uint16_t message_index = (can_handle->pRxMsg->Data[1] << 8) + can_handle->pRxMsg->Data[2];
switch (message_index)
{
case 0x1008:
device_name_[0] = can_handle->pRxMsg->Data[4];
device_name_[1] = can_handle->pRxMsg->Data[5];
device_name_[2] = can_handle->pRxMsg->Data[6];
device_name_[3] = can_handle->pRxMsg->Data[7];
device_name_[4] = '';
break;
case 0x100A:
software_version_[0] = can_handle->pRxMsg->Data[4];
software_version_[1] = can_handle->pRxMsg->Data[5];
software_version_[2] = can_handle->pRxMsg->Data[6];
software_version_[3] = can_handle->pRxMsg->Data[7];
software_version_[4] = '';
break;
case 0x1018:
serial_number_ = can_handle->pRxMsg->Data[4] << 24 | can_handle->pRxMsg->Data[5] << 16 |
can_handle->pRxMsg->Data[6] << 8 | can_handle->pRxMsg->Data[7];
break;
case 0x2100:  // DC PowerCube status
power_cube_status_ = can_handle->pRxMsg->Data[4];
io_status_bit_ = can_handle->pRxMsg->Data[5];
dip_switch_status_bit_ = can_handle->pRxMsg->Data[6];
break;
case 0x2111:  // Grid voltage, current, current limit
grid_voltage_ = (can_handle->pRxMsg->Data[4] << 8) + can_handle->pRxMsg->Data[5];
grid_current_ = can_handle->pRxMsg->Data[6];
grid_current_limit_ = can_handle->pRxMsg->Data[7];
break;
case 0x2112:  // Generator frequency, RPM
generator_freq_ = (can_handle->pRxMsg->Data[4] << 8) + can_handle->pRxMsg->Data[5];
rpm_ = (can_handle->pRxMsg->Data[6] << 8) + can_handle->pRxMsg->Data[7];
break;
case 0x2113:  // Generator current
gen_current_phase1_ = can_handle->pRxMsg->Data[4];
gen_current_phase2_ = can_handle->pRxMsg->Data[5];
gen_current_phase3_ = can_handle->pRxMsg->Data[6];
gen_current_limit_ = can_handle->pRxMsg->Data[7];
break;
case 0x2114:  // Load percentage
grid_load_ = can_handle->pRxMsg->Data[4];
generator_load_ = can_handle->pRxMsg->Data[5];
dc_output_load_ = can_handle->pRxMsg->Data[6];
break;
case 0x2151:  // Battery type & charger state
battery_type_ = can_handle->pRxMsg->Data[4];
charger_state_ = can_handle->pRxMsg->Data[5];
break;
case 0x2152:  // DC output voltage & DC slave voltage
dc_output_voltage_ = (can_handle->pRxMsg->Data[4] << 8) + can_handle->pRxMsg->Data[5];
dc_slave_voltage_ = (can_handle->pRxMsg->Data[6] << 8) + can_handle->pRxMsg->Data[7];
break;
case 0x2153:  // DC output current & DC output current limit
dc_output_current_ = (can_handle->pRxMsg->Data[4] << 8) + can_handle->pRxMsg->Data[5];
dc_output_current_limit_ = (can_handle->pRxMsg->Data[6] << 8) + can_handle->pRxMsg->Data[7];
break;
case 0x21A0:  // Temperature sensor
temp_sens_BTS_ = can_handle->pRxMsg->Data[4];
temp_sens_intern1_ = can_handle->pRxMsg->Data[5];
temp_sens_intern2_ = can_handle->pRxMsg->Data[6];
temp_sens_intern3_ = can_handle->pRxMsg->Data[7];
break;
case 0x21A1:
break;
}
}
}

WhisperConnectDevice 是 DCPowerCube 的基类。

我很想得到一些关于如何处理这个问题的反馈。

无论是否引入多态性，您都必须将外部提供的类型编号(ID)映射到代码，因此您始终需要介于两者之间的一些结构。

您的候选人是：

  1. 一段if语句可能if-else-if......
  2. switch语句(如果值可表示)
  3. 某种查找表(数组、关联映射、其他...

你已经有了if，但可以通过if-else-if来改进。 这通常被认为是最丑陋的高维护潜力编码热点方法。编码热点，因为所有新 ID 都返回到此代码块。

我还注意到在这种情况下，对于某些nn，您的所有范围都0xnn0包含0xnnF，因此您至少可以通过减少低4位来简化：

auto whisper_connect_type = whisper_connect_id_ >> 4;

然后，您的switch选项将简化为：

switch(whisper_connect_type) {
case 0x58: WIBDevice(); break;
case 0x59: BMSSDevice(); break;
case 0x5B: DCPowerCubeDevice(); break;
case 0x5C: ACPowerCubeDevice(); break;
case 0x70: WIBHeartBeatDevice(); break;
default: HandleUnknownDeviceIDError(whisper_connect_id_); break;
}

注意：我强烈建议使用一些代码来处理不受支持的 ID。我的建议是抛出一个例外或导致终止的东西。break;是为了完整性。我不认为你是从一个未知的ID回来的。

另一种方法是定义关联映射：

#include <iostream>
#include <unordered_map>
#include <memory>
class WhisperHandler {
public:
virtual void HandleWhisper() const = 0 ;
virtual ~WhisperHandler() {}
}; 
class WhisperHandlerWIBDevice : public WhisperHandler {
public:
void HandleWhisper() const override {
std::cout << "Handler WIBDevice...n";
}
} ;
int main() {
std::unordered_map<unsigned,std::unique_ptr<const WhisperHandler>> handlers;
//...
std::unique_ptr<const WhisperHandler> handler(std::make_unique<const WhisperHandlerWIBDevice>());
std::pair<const unsigned , std::unique_ptr<const WhisperHandler> > pair({0x5B,std::move(handler)});
handlers.insert(std::move(pair));
//...
{
const auto &chandlers=handlers;
auto handlerit(chandlers.find(0x5B1));
if(handlerit!=chandlers.end()){
handlerit->second->HandleWhisper();
}else{
//ERROR - UNKNOWN HANDLER.
}
}
return 0;
}

但是，我建议只有当您允许从应用程序的不同模块或通过动态加载在加载时注册自己的库来注册处理程序时，您才能获得所有这些多态机制的投资回报。

如果它是一个单一的项目应用程序(看起来是)，那么switch表调度可能会正常工作。

由于应用程序倾向于使用某种 ID 进行通信，因此在实践中，当它需要获取 ID，将其映射到多态处理程序，然后调用处理程序时，OO 可能会开始看起来很麻烦。从逻辑上讲，您已经完成了两次 ID 到逻辑映射！

脚注：敲除最低 4 位的技巧与这些方法有些不同，如果较低的 4 位与确定处理程序相关，(当然)有点脆弱。

相关文章：