Quoth Carlos R. Mafra,

Perhaps it's just me, but I can't really get what the function does
from just reading its name: wAppIconFor(). I'm no native speaker, but
the "For" suffix confuses me. I wonder if wAppIconOf() would be more
"correct" -- ignoring the CamelCase issue :-)

You're probably right. I chose the name to match the existing function wWindowFor() since it does a similar thing.

I think there is a better name which clearly states its purpose.
I'm bad in choosing names, though.

Function names are all over the place. We have some in Camel case, some all lowercase and I'm not sure of the logic behind some starting with the w prefix and others not.

A more general discussion on naming may be worthwhile. I'm happy to change the name of the new function to something which we can agree on. If I change it, though, I think I should change wWindowFor() as well. Or even everything (mwahahaha!)...

I guess that justifies having some comment on top of the function
to state its purpose? I know it's a small function, but I find it
nicer to have an overview of what the function is supposed to do
before reading it.

No problem, and again if wAppIconFor() needs a comment then wWindowFor() does too.
From 8d696a3776c2b7df4316232ae31cf3be39ec0dc9 Mon Sep 17 00:00:00 2001
From: Iain Patterson <[email protected]>
Date: Tue, 30 Oct 2012 09:10:14 -0700
Subject: [PATCH] Brief documentation on wAppIconFor() and wWindowFor().

Brief comment on what wAppIconFor() and wWindowFor() do, as it
may not be immediately obvious.

Given a raw X11 Window they will find the AppIcon (or WWindow,
respectively) associated with that window.  Thus they can map an
X11 object to a Window Maker internal object.
---
 src/appicon.c | 1 +
 src/window.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/appicon.c b/src/appicon.c
index 5d654de..cc2f7d8 100644
--- a/src/appicon.c
+++ b/src/appicon.c
@@ -1021,6 +1021,7 @@ static void remove_from_appicon_list(WScreen *scr, 
WAppIcon *appicon)
        appicon->next = NULL;
 }
 
+/* Return the AppIcon associated with a given (Xlib) Window. */
 WAppIcon *wAppIconFor(Window window)
 {
        WObjDescriptor *desc;
diff --git a/src/window.c b/src/window.c
index 4691021..199613c 100644
--- a/src/window.c
+++ b/src/window.c
@@ -141,6 +141,7 @@ static void appearanceObserver(void *self, WMNotification * 
notif)
 }
 
 
+/* Return the WWindow associated with a given (Xlib) Window. */
 WWindow *wWindowFor(Window window)
 {
        WObjDescriptor *desc;
-- 
1.7.11.4

Reply via email to