Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update uri.hpp #1131

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

xiaoxin-mz
Copy link

@xiaoxin-mz xiaoxin-mz commented Mar 25, 2024

std::string get_port_str() const {
std::stringstream p;
p << m_port;
return p.str();
}

if m_port == 9999, sometimes p.str() returns "9,999",i don't know why. But I can use std::to_string () instead

std::string get_port_str() const {
        std::stringstream p;
        p << m_port;
        return p.str();
    }
if m_port == 9999, sometimes p.str() returns "9,999",i don't know why. but change to use std::to_string() is ok.
@xiaoxin-mz
Copy link
Author

xiaoxin-mz commented Mar 25, 2024

std::locale("")
It seems that this function affects the result of std::stringstream. It is better to use std::to_string ().

it should be function get_port_str
Copy link

@Imran-imtiaz48 Imran-imtiaz48 left a comment

Choose a reason for hiding this comment

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

Overview

The modifications in the code improve the functionality by simplifying the string conversion process for port numbers and ensuring consistency in the format of the returned strings. Here are the key points of the review:

  1. Consistency and Simplification:

    • The changes improve the consistency in string conversion across the methods by using std::to_string for port conversion.
    • The updated code simplifies the logic by removing the use of std::stringstream where not necessary.
  2. Readability and Maintainability:

    • The refactoring enhances readability by making the code more straightforward.
    • The usage of std::to_string directly in return statements reduces the lines of code, making it easier to maintain.

Improvement Suggestions

  1. Optimization of Host and Port Concatenation:
    • To further simplify and optimize the code, consider using std::to_string consistently for the host and port concatenation.
if (m_port == (m_secure ? uri_default_secure_port : uri_default_port)) {
    return m_host;
} else {
    return m_host + ":" + std::to_string(m_port);
}
  1. Documentation and Comments:
    • Add comments to explain the logic, particularly for why certain conversions are used. This helps in maintaining the code in the long term.
// Return the host if the port is default for the scheme, otherwise return host:port
if (m_port == (m_secure ? uri_default_secure_port : uri_default_port)) {
    return m_host;
} else {
    return m_host + ":" + std::to_string(m_port);
}
  1. Consistency in String Conversion Methods:
    • Ensure that std::to_string is used throughout the codebase for consistency, unless there is a specific need for std::stringstream.

Summary

The changes made are effective in improving the code by simplifying the string conversion and making it more readable and maintainable. Implementing the suggested improvements will further enhance the code's clarity and performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants