Am 05.02.2008 um 18:19 schrieb Florian Westphal:
Dana Dahlstrom dana+70@cs.ucsd.edu wrote:
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?
No. But in this case i've decided not do do that. There are no other users of this functionality and moving it to channel.c requires to export a new function. Having it in irc-channel.c means we can keep it static, and gcc then even removes the function call.
Semms to be the correct way: local ("static") helper functions do belong in the module from which they are called.
Wherever the code lives, I noticed there's a way to make the loop slightly more elegant:
I usually try to avoid assignments inside conditionals; and here is why:
irc-channel.c: In function 'part_from_all_channels': irc-channel.c:57: warning: suggest parentheses around assignment used as truth value
Easily silenced, but this turns
- while (cl2chan) {
- while (cl2chan = Channel_FirstChannelOf(target)) {
into while ((cl2chan = Channel_FirstChannelOf(target))) {
I'm not entirely sure which is more elegant 8-)
Since you felt strong enough about it to actually send a patch i'll apply this (unless vetoed by Alex).
(the generated code is the same, so this really is only about CodingStyle)
I'm not sure about this one: in the first place, the variant suggested by Dana looks cleaner, but the "double braces" required to calm down gcc make it ugly. So its ... a little bit ugly either way :-)
So probably the code is more readable the way it is at the moment?
Regards Alex