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.

Sign up for GitHub

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

43 feature create serversocket referring to config class #47

Merged
merged 3 commits into from
Feb 5, 2024
Merged

43 feature create serversocket referring to config class #47

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e86b31
startUpServerSockets creates and strats ServerSocket up in main.cpp
ilovenoah Feb 5, 2024
fa88a4b
Exit with status 1 if starting server socket up is failed
ilovenoah Feb 5, 2024
c2ccc70
Logging out started up servers
ilovenoah Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,26 @@
#include "ServerSocket.hpp"
#include "loop.hpp"

// int main(int argc, const char *argv[]) {
// Config config;

// config.setServers(argc, argv);
// startServerLoop(config);
// return (0);
// }
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"; }
Comment on lines +11 to +12
Copy link

Choose a reason for hiding this comment

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

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.

ServerSocket ss(ipAddr, port);
if (ss.init() == false) { return false; }
std::cout << "Start up server: " << ipAddr << ":" << port << std::endl;
Copy link

Choose a reason for hiding this comment

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

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));
}
return true;
Comment on lines +6 to +18
Copy link

Choose a reason for hiding this comment

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

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.

}

int main(int argc, const char *argv[]) {
ServerSocket ss("127.0.0.1", "8080");
std::map<int, ServerSocket> ssmap;
Config config;

config.setServers(argc, argv);
ss.init();
std::map<int, ServerSocket> ssmap;
ssmap.insert(std::pair<int, ServerSocket>(ss.getFd(), ss));
bool test = loop(ssmap, config);
(void)test;
if (startUpServerSockets(ssmap, config) == false) { exit(1); }
Copy link

Choose a reason for hiding this comment

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

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.

loop(ssmap, config);
}
Loading