Skip to content

Commit

Permalink
兼容c++11
Browse files Browse the repository at this point in the history
  • Loading branch information
@xia-chu
xia-chu committed Dec 7, 2024
1 parent 54a63d9 commit a555e68
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/Network/UdpServer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,28 @@
#ifndef TOOLKIT_NETWORK_UDPSERVER_H
#define TOOLKIT_NETWORK_UDPSERVER_H

#if __cplusplus >= 201703L
#include <array>
#include <string_view>
#endif
#include "Server.h"
#include "Session.h"

namespace toolkit {

class UdpServer : public Server {
public:
#if __cplusplus >= 201703L
class PeerIdType : public std::array<char, 18> {
#else
class PeerIdType : public std::string {
#endif
public:
#if __cplusplus < 201703L
PeerIdType() {
resize(18);
}
#endif
bool operator==(const PeerIdType &that) const {
return as<uint64_t>(0) == that.as<uint64_t>(0) &&
as<uint64_t>(8) == that.as<uint64_t>(8) &&
Expand Down Expand Up @@ -91,7 +103,11 @@ class UdpServer : public Server {

private:
struct PeerIdHash {
#if __cplusplus >= 201703L
size_t operator()(const PeerIdType &v) const noexcept { return std::hash<std::string_view> {}(std::string_view(v.data(), v.size())); }
#else
size_t operator()(const PeerIdType &v) const noexcept { return std::hash<std::string> {}(v); }
#endif
};
using SessionMapType = std::unordered_map<PeerIdType, SessionHelper::Ptr, PeerIdHash>;

Expand Down

2 comments on commit a555e68

@alexliyu7352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compatible with C++11

TRANS_BY_GITHUB_AI_ASSISTANT

@alexliyu7352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review for src/Network/UdpServer.h:

Code Review: Patch for UdpServer.h in ZLToolKit

Summary

This patch modifies the UdpServer.h file to improve compatibility with C++11 and earlier versions. It introduces conditional compilation based on the C++ standard version (__cplusplus) to handle differences in available standard library features. The primary change is the adaptation of PeerIdType to use std::string in C++11 and earlier, while using std::array in C++17 and later.

Detailed Feedback

Code Overview

The patch addresses the lack of C++11 compatibility in the original code by conditionally using std::array<char, 18> for PeerIdType in C++17 and later, and falling back to std::string for older standards. It also adjusts the PeerIdHash struct to use the appropriate std::hash specialization for each case. The addition of a default constructor for PeerIdType when using std::string ensures proper initialization.

Strengths

  • Improved C++ Standard Compatibility: The primary strength is the improved compatibility with C++11 and earlier versions. This makes the code more portable and usable in a wider range of environments.
  • Conditional Compilation: The use of #if __cplusplus >= 201703L provides a clean and efficient way to handle the differences between C++ standards.
  • Clear and Concise Changes: The changes are well-structured and easy to understand.

Areas for Improvement

1. PeerIdType Design and Potential Inefficiency

  • Issue: While the patch successfully addresses C++11 compatibility, the choice of std::string as a fallback might introduce performance overhead compared to std::array. std::string involves dynamic memory allocation, which can be slower than direct access to a fixed-size array. The hash function for std::string also involves more computation than a custom hash function designed for the fixed-size array.
  • Suggestion: Consider a custom fixed-size array implementation even for C++11 compatibility. This would maintain consistent performance across all C++ standards. A custom hash function tailored to this array would further optimize performance.
  • Example:
#if __cplusplus >= 201703L
    class PeerIdType : public std::array<char, 18> {
        // ...
    };
#else
    class PeerIdType {
    private:
        char data_[18];
    public:
        // ... methods to access data_ ...
        bool operator==(const PeerIdType &that) const {
            return memcmp(data_, that.data_, 18) == 0;
        }
        // ... custom hash function ...
    };
#endif

2. Error Handling and Robustness

  • Issue: The code lacks explicit error handling. Network operations can fail, and the code should gracefully handle such situations.
  • Suggestion: Add error checking to network operations (socket creation, reading, etc.) and implement appropriate error handling mechanisms (e.g., logging errors, returning error codes, throwing exceptions).

3. Documentation

  • Issue: While the code has comments, they are primarily translations. More detailed comments explaining the logic and design choices would improve readability and maintainability.
  • Suggestion: Add more detailed comments explaining the rationale behind the conditional compilation, the choice of std::array vs. std::string, and the implementation of the PeerIdType and PeerIdHash.

Conclusion

The patch effectively improves C++ standard compatibility. However, the performance implications of using std::string as a fallback should be carefully considered. Implementing a custom fixed-size array and adding robust error handling would significantly enhance the code's quality and reliability. Finally, improving the documentation would make the code easier to understand and maintain.

TRANS_BY_GITHUB_AI_ASSISTANT

Please sign in to comment.