43 feature create serversocket referring to config class #47
Conversation
WalkthroughThe recent update involves refactoring the Changes
Assessment against linked issues
Poem
🐇✨ Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
ilovenoah
deleted the
43-feature-create-serversocket-referring-to-config-class
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| static bool startUpServerSockets(std::map<int, ServerSocket> &ssmap, Config &config) { | ||
| std::vector<Server> servers(config.getServers()); | ||
| for (std::vector<Server>::iterator iter = servers.begin(); iter != servers.end(); ++iter) { | ||
| std::string ipAddr = iter->getIpAddress(); | ||
| std::string port = iter->getPort(); | ||
| if (ipAddr.empty() == true) { ipAddr = "0.0.0.0"; } | ||
| if (port.empty() == true) { port = "8000"; } | ||
| ServerSocket ss(ipAddr, port); | ||
| if (ss.init() == false) { return false; } | ||
| std::cout << "Start up server: " << ipAddr << ":" << port << std::endl; | ||
| ssmap.insert(std::pair<int, ServerSocket>(ss.getFd(), ss)); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
The startUpServerSockets function lacks error handling for the case where ServerSocket initialization (ss.init()) fails. It returns false immediately, which could leave resources partially initialized if an error occurs after some successful initializations. Consider cleaning up or logging detailed error information before returning.
| if (ipAddr.empty() == true) { ipAddr = "0.0.0.0"; } | ||
| if (port.empty() == true) { port = "8000"; } |
There was a problem hiding this comment.
Hardcoding default values ("0.0.0.0" for IP address and "8000" for port) within the startUpServerSockets function reduces flexibility. Move these defaults to a configuration class or a constants file to facilitate easier adjustments and maintainability.
| if (port.empty() == true) { port = "8000"; } | ||
| ServerSocket ss(ipAddr, port); | ||
| if (ss.init() == false) { return false; } | ||
| std::cout << "Start up server: " << ipAddr << ":" << port << std::endl; |
There was a problem hiding this comment.
Directly printing to std::cout within startUpServerSockets limits the flexibility of logging. Consider using a logging framework or mechanism that supports different log levels and destinations.
| ssmap.insert(std::pair<int, ServerSocket>(ss.getFd(), ss)); | ||
| bool test = loop(ssmap, config); | ||
| (void)test; | ||
| if (startUpServerSockets(ssmap, config) == false) { exit(1); } |
There was a problem hiding this comment.
The use of exit(1); directly in the main function upon failure of startUpServerSockets is abrupt. It's better to clean up resources or provide a clear error message before exiting. Additionally, consider using a named constant for the exit code to improve readability.
This PR is to resolve #43 .
Summary by CodeRabbit