Ten lines seems very small. Some of my functions have gotten smaller as I've worked on them, but the average is more than ten lines. I asked for advice about the code here a few months ago and no one said anything about the functions being too long.
Aesthetic criticism is generally discouraged, because it is so subjective. Thus, I'm not telling Spirrwell "your functions are too long", just that "I've generally found that shorter functions make for more readable code, and readability is really nice."
As an example (I hope you don't mind, I'll remove if you do):
[source lang="cpp"]unsigned char GiveOne ()
{
if (index >= packetLength) {
throw failure("ReceiveBuffer::Give -- out of data");
}
unsigned char byte = buf[index];
++index;
return byte;
}[/source]
This function is extremely readable, all statements are at the same level of abstraction, and a developer who knows little to nothing about your system can immediately understand the logic. On the other hand:
[source lang="cpp"]
sock_type connect2CMW (cmwa_config_data const& configData) {
sock_type sockfd = cmw::getSocket(SOCK_STREAM);
#if 0
sockaddr_in SockAddr;
SockAddr.sin_family = AF_INET;
SockAddr.sin_port = htons(56791);
SockAddr.sin_addr.s_addr = inet_addr("10.0.0.184");
if (::connect(sockfd, (sockaddr*)(&SockAddr), sizeof(SockAddr)) == 0) {
return sockfd;
}
#else
for (;;) {
#ifdef CMW_WINDOWS
int const basePort = 56791;
#else
int const basePort = 56789;
#endif
::std::string node(configData.localhost ? "127.0.0.1" : "www.webEbenezer.net");
for (int j = 0; j < 6; ++j) {
addrinfo *res;
getaddrinfo_wrapper(node.c_str(), basePort + j % 2, &res);
if (::connect(sockfd, res->ai_addr, res->ai_addrlen) == 0) {
freeaddrinfo(res);
#ifdef SYSLOG_AVAILABLE
syslog(LOG_INFO, "connected to CMW on port %d", basePort + j % 2);
#endif
return sockfd;
}
// 111 is connection refused and 113 is no route to host.
printf("connect() failed on port %d. errno is %d\n"
, basePort + j % 2, GetError());
freeaddrinfo(res);
}
#ifdef CMW_WINDOWS
Sleep(configData.sleepseconds * 1000);
#else
sleep(configData.sleepseconds);
#endif
}
#endif
}[/source]
this one has a few different levels of abstraction (low level OS code, med-low level application code?), is quite a bit longer, and performs several steps in an apparently complex operation, but I wouldn't be able to readily identify the boundaries or proper high level names for those steps without being more familiar with the domain. Again, I'm not saying it's bad, incorrect, or anything like that, just that subjectively, I am less able to grok this than I (think) I would be if you were to do something like:
[source lang="cpp"]
sock_type connect2CMW (cmwa_config_data const& configData) {
sock_type sockfd = cmw::getSocket(SOCK_STREAM);
//Not sure if you sometimes need to enable that disabled code? Still under development?
for (;;) {
::std::string node(configData.localhost ? "127.0.0.1" : "www.webEbenezer.net");
int port = GetBasePort();
for (int j = 0; j < 6; ++j, port += j%2) {
addrinfo *res;
getaddrinfo_wrapper(node.c_str(), port, &res);
if (::connect(sockfd, res->ai_addr, res->ai_addrlen) == 0) {
freeaddrinfo(res);
AttemptLogCMWConnection(port);
return sockfd;
}
freeaddrinfo(res);
}
SleepUntilRetry(configData);
}
void AttemptLogCMWConnection(int port) {
#ifdef SYSLOG_AVAILABLE
syslog(LOG_INFO, "connected to CMW on port %d", port);
#endif
}
void PrintCMWConnectionError(intport) {
// 111 is connection refused and 113 is no route to host.
printf("connect() failed on port %d. errno is %d\n"
, port, GetError());
}
void SleepUntilReconnect(cmwa_config_data const& configData) {
#ifdef CMW_WINDOWS
Sleep(configData.sleepseconds * 1000);
#else
sleep(configData.sleepseconds);
#endif
}
int GetBasePort() {
#ifdef CMW_WINDOWS
return 56791;
#else
return 56789;
#endif
}
[/source]
I may have mangled the logic, and again, I am not implying that this is better (unless your compiler is really good, it may be slower for instance). Hope this illustrates what I mean better though