I will have to disagree with the comments about you should break up functions if they exceed x amount of lines. One example (albeit, it's not pretty in any regards):
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34
|
template<class T>
void vp::list<T>::sort() {
if (size() <= 1)
return;
for (int merges = 0, k = 1; merges != 1; k *= 2) {
node *p = firstNode, *q = p;
for (int qsize = merges = 0; p; merges ++, p = q) {
for (qsize = 0; qsize < k && q; qsize ++, q = q->next);
while (qsize && q) {
if (q->value <= p->value) {
if (q->next) {
q = q->next;
q->prev->insert_before(p);
}
else {
q->insert_before(p);
q = NULL;
lastNode = lastNode->prev;
}
if (p == firstNode)
firstNode = firstNode->prev;
qsize --;
}
else
p = p->next;
}
}
}
}
|
This was about as short as I could possibly make this function, and if you can see, it still is hard to read. Granted, I could change some variable names, separate some things on to their own line, but this is basically as simple as this algorithm gets. The is another version of the merge sort which relies on a separate split function, and possibly a merge function, which would reduce the number of lines, but then you detract from the overall readability of the program, in my eyes.
I am still in the process of commenting all of my functions (I've only gotten to do the Node methods) and need to rewrite some of them (the above being one) for increased readability and performance issues. However, no matter what your general rule of thumb is, I believe that there is always going to be exceptions. Also, you have to understand that there comes a point where increasing readability can really take away from the performance. It may seem small at first, but running my list through the sort algorithm takes only 2x longer than it does to add 1million items to said list. I'm fairly happy with the performance, however, I would like to improve it, per a rewritten function that cire has presented me.
I'm also looking into the recursive version, which in his example, knocks about 200ms off of the time to run the entire function.
I'm not above putting comments in functions. Heck, before I sat down with this list again, I had a comment for every line, letting my know exactly what each line was doing and why. It makes the code harder to read, for some people, but there is also no doubt as to why everything is the way it is. My new approach is going to be to generate a paragraph or so before each function to explain all of that, the algorithm used, if there is one, the reason for choosing the method I used, and params and return values. I'm also looking into adding post and pre conditions for my code to express when and how a function should be used.
I'm also looking into how I want to structure each of my functions, as well as looking into possible ways to improve overall speed of everything. I've learned the hard way, more recently than before, that less lines doesn't mean faster code. However, there comes a time when you have to decide what's more important. For all intensive purposes, I want to completely optimize this class to get a complete and thorough understanding of why this and not that. I also want to share my code with others to demonstrate that, even though my code has several hundred lines (well over 1000 by the time I'm completely done), it has speed and efficiency in mind. My code will also, in the future, be very easily read and understood, even with the extra lines in it.
I'm interested to see the people's opinion on my function who said x number of lines is the max that should be in a function. What do you believe would make this more efficient, easier to read (aside from replacing the names and breaking down the for loop)? I'm also interested to see where you stand on other seemingly long functions that deal with x algorithm when y algorithm would have been shorter to write, but overall slower.