The Nightstar Zoo

Here's a trivial piece of code, written two ways.

Which is better? Why? (Disconsider error trapping; it has been removed for clarity. I'm not even sure Edit_SetText is the correct macro name. But *you* can read it and understand what it does.)

Snippet One

Snippet Two

I have some strong opinions about this, but they didn't really become clear to me until I wrote this. Interesting.... I'll hold them for now. What do you guys think?

I prefer the latter - it keeps things together by what they're talking about, and not how they're doing things -- after all, you're probably going to change how they do things.

(edit: get which is which straight!)

Vorn

I usually use the first for readability--much easier to read, a little less convenient to add to--but only when there are only a few different lines for each item (in #2 form). Otherwise, it seems best to go with 2.

Ideally little blocks like that can be seperated off into a small macro or inline function, which throws out the question entirely.

_________________
delete this;

Code snippet two may seem worse than #1, but that is because the second snippet is screaming for optimization. That code is not finished. Here is what I would feel compelled to do with that code (again, assuming that it was more than a few lines of code):



...this is not to say that you couldn't optimize block 1, as well, however:


Thoughts? Observations? I submit that both of these blocks are, in fact, better than their forbears... even if block 1 optimized to more code.

Raif, you're on the right track... don't look at what's there. Look at what's not there, and infer. Assume that this code, for example, is buried in 100,000 lines of windows code, etc.

We're still not done. How would you optimize this code? Assume that it is one of 20 such dialogs in an application, some of which have 2-3 controls, other with 40....

None of the above. I'd probably have an object that did all 4 lines inside of its method. (aka tell the dialog to go show itself)

In practice, this would be roughly equal in function to the second except far less cluttered.

From a functional standpoint, The first you're throwing a bunch of time upfront to initialize it before getting any user input too. This seems like the opposite of ideal way of doing the processing side. (just like TI's paren style calculation throwing all the calculation at the end is not an ideal way to operate a calculator, and HP RPN where it calculates as you enter it is better.)

_________________
KazLJ: Politics, Gaming, Technology, and other wierdness

Actually, I take it back. I just realized that the "optimization" of code block 1 is pure nast. It's longer, less readable, and forced me to dramatically increase the complexity of the task.

I'm not just creating maps and vectors, I'm passing them in interfaces. This means anything that can see that interface now depends on <map> and <vector> being defined, etc.

Nast. Pure nast.

Premature optimization. Or perhaps optimization at the expense of readability.

I think this one would get its benefits solely from the code already being in the cache due to the loops, assuming lots of code for each dialog box in the other version. You're also sacrificing memory for speed here. (you don't REALLY need to keep the data around in the other example, but you do in this one.)

You really should tell the windows to go ____ themselves.

Edit: The spelling nazi in me made me do it.

_________________
KazLJ: Politics, Gaming, Technology, and other wierdness

Darn it, you keep beating me to posts.

_________________
KazLJ: Politics, Gaming, Technology, and other wierdness

In the general case, what I would've said has mostly already been said. Take all that crap out of its own function and put it into subroutines, or better yet, objects.

In that specific case, my only caution is that there may be important business reasons to have all fields initialized before enabling/showing any of them. If that's the case, you're restricted to option 1 or an equivalent.

And the equivalents can still be fairly similar to Option 2 revised. They can be done with similar objects as well. Just move a couple of those things out of Show() and stick them in __init__() then break up the init from the show in the implementation code. Your message seems to note that enabling and showing can still be tied together.

_________________
KazLJ: Politics, Gaming, Technology, and other wierdness

Okay, Kaz and I posted at the same time, and he basically preempted the "we're still not done" portion of the show.

The VERY interesting implecation of all this is that...

Object-oriented programming roxxors and function-oriented code sucks it. WOOT! Go OO!

Okay, back to calm reason.

The thing I'm see is that if you start grouping methods together by object rather than by function, you start to see the natural emergence of objects, and therefore, classes in the code.



Now, that's not really much better that the first round of optimization. If this were a Win32 C++ program without MFC, I would have left it at the first optimization.

We've started to reach equilibrium with regard to the EditWindow objects: Each window has a caption and a resource id that can be converted into an HWND; with every window we want to perform the same three actions: set text, enable, show. Any more gains with this program will probably be made at a higher level.

Let's go back to the InitLoginDialog() method. We're passing in hDlg, about which there are two significant things we observe: first, it's the handle to an instance of a dialog, and second, it's used to convert dialog resource ID's into HWNDs. Okay, that's where it comes from and what we use it for... but, so what?

Think about that first implication. It's the unique identifier of an instance of a dialog class. I said a few posts back that this system has 20 or so of these dialogs. What if we could turn our dialog function into a dialog class member function?

I'm going to stop here. This way lies MFC/ATL/Swing/TCL/Qt/wxWidgets or whatever windowing framework you care to use. You could take the parent function and make it a dialog method, bind the three controls to a custom class that inherits from an existing Edit class, etc. Eventually you end up with code where the binding of the resource id to HWNDs is done for you, and all OnInitDialog() has to do is call Init() on its member objects.

This code discussion optimized the code until we needed a Windowing framework to go any further. What I find interesting is that having a windowing framework at hand does not necessarily lead you start with any such intelligently optimized code.

The code I posted first (option #1, grouped by method) was taken directly from my project today, from inside an MFC class. (I removed all the CDialog stuff to keep the snippet uncluttered.)

Iddint dat inneresting....

Of course.

I find that the first impulse of programmers writing code inside a windowing framework like VB is to end up with lots, and lots of parallel structures. Cut and paste coding, really.

What irks me when I work in VB is that it seems to actively push towards this type of coding, and makes it harder to factor the parallel structures out.

_________________
KazLJ: Politics, Gaming, Technology, and other wierdness

Iknewit!

Kaz: Indeed. Cut and paste makes me twitch.

_________________
delete this;

This is a trait of any object-based (but not truly object-oriented) language. You end up having to apply the same decorations to the end classes, and it is harder to push common functionality up into higher classes.

Note that you *can* do it in VB... even in the older versions; but when you're done you have code that looks a little arcane and more importantly, violates some of the idioms of VB programming. In other words, good design is bad style.

Heh. "Good design considered harmful" would make a great usenet flamebait topic.