On 2008-02-05 at 14:40 +0100, Florian Westphal wrote:
Applied -- thanks for this work. Also thanks for adding test cases -- this is really much appreciated.
It was my pleasure! I wrote the tests first, actually---in conformance with the XP methods we're trying to teach in the class. :)
I like refactoring out part_from_all_channels as a function, but I wonder if it doesn't belong in channel.c instead of irc-channel.c. It looks to me like channel.c contains a bunch of utilities while every function in irc-channel.c handles an actual IRC message and accepts a CLIENT* and a REQUEST* as parameters. Is this convention the project has decided to abandon?
Wherever the code lives, I noticed there's a way to make the loop slightly more elegant:
diff -u -p -r1.42 irc-channel.c --- src/ngircd/irc-channel.c 5 Feb 2008 13:31:50 -0000 1.42 +++ src/ngircd/irc-channel.c 5 Feb 2008 16:15:24 -0000 @@ -51,16 +51,13 @@ static char UNUSED id[] = "$Id: irc-chan static bool part_from_all_channels(CLIENT* client, CLIENT *target) { - CL2CHAN *cl2chan = Channel_FirstChannelOf(target); + CL2CHAN *cl2chan; CHANNEL *chan;
- while (cl2chan) { + while (cl2chan = Channel_FirstChannelOf(target)) { chan = Channel_GetChannel(cl2chan); assert( chan != NULL ); Channel_Part(target, client, Channel_Name(chan), Client_ID(target)); - - /* next */ - cl2chan = Channel_FirstChannelOf(target); } return CONNECTED; }