?

Log in

No account? Create an account

The stupid, it burns! - Journal of Omnifarious

Jan. 25th, 2006

07:56 pm - The stupid, it burns!

Previous Entry Share Next Entry

I'm dealing with a huge body of code. I'm estimating that it's about 5-10 times larger than it needs to be to get the task at hand done. Seriously.

For example, there's a lot of XML parsing going on. A whole pile of it. For each and every element or attribute name, there are two constants and one #define. The constants are painstakingly initialized one at a time by a big block of code. The constants and #define names are each at least 4 times longer than the string they stand in for. These names are religiously used throughout the rest of the code.

AIGH!

*sigh*

Small example, lest you disbelieve me (keep in mind that this code is actually scattered in several different places and not conveniently stuck together):

#define SYSTEM_VSAM_STR_ELEM_VSAM_REQUEST_FIELDS                  "RequestFields"
XMLCh* System_VsamLayerUtils::DOM_SYSTEM_VSAM_STR_ELEM_VSAM_REQUEST_FIELDS				= NULL;
DOM_SYSTEM_VSAM_STR_ELEM_VSAM_REQUEST_FIELDS	= XMLString::transcode(SYSTEM_VSAM_STR_ELEM_VSAM_REQUEST_FIELDS);
tcDeleteTranscode(DOM_SYSTEM_VSAM_STR_ELEM_VSAM_REQUEST_FIELDS);

And the worst thing is, all the XML parsing code is using the LONG_NAMES instead of simple strings, and I'm trying to figure out what various pieces of XML cause the program to do. This means I have to be flipping all around the program hunting down the definitions of these stupid constants.

P.S. Sorry about the late edits and overly wide lines. I was pressed for time as the coffee shop I was in was closing. I should've made the entry private or something then made it public later. Though, some of the styles are worse than others if people make their entries too wide.

Current Mood: [mood icon] aggravated

Comments:

[User Picture]
From:sfllaw
Date:January 26th, 2006 03:58 am (UTC)
(Link)
Please please please wrap at 72 characters!
(Reply) (Thread)
[User Picture]
From:noveldevice
Date:January 26th, 2006 04:16 am (UTC)
(Link)
Seconded.
(Reply) (Parent) (Thread)
[User Picture]
From:omnifarious
Date:January 26th, 2006 06:07 am (UTC)
(Link)

Sorry about that. :-)

(Reply) (Parent) (Thread)
[User Picture]
From:scottscidmore
Date:January 26th, 2006 04:26 am (UTC)
(Link)
Just need an Apple Cinema HD Display.
(Reply) (Parent) (Thread)
[User Picture]
From:sfllaw
Date:January 26th, 2006 04:33 am (UTC)
(Link)
Large displays are for many thin terminals, not for one wide one!
(Reply) (Parent) (Thread)
[User Picture]
From:scottscidmore
Date:January 26th, 2006 04:47 am (UTC)
(Link)
I thought large displays were for seeing more of the map when playing FreeCiv...

(Reply) (Parent) (Thread)
[User Picture]
From:omnifarious
Date:January 26th, 2006 06:06 am (UTC)
(Link)

Sorry about that. :-)

(Reply) (Parent) (Thread)
[User Picture]
From:codesamurai
Date:January 26th, 2006 04:20 am (UTC)
(Link)
Heh, naming convention taken too far. Seems like the convention was designed to group all constants according to specific functionality. It doesn't seem like it would help much, they're hard to code review and they're not type safe. *shrugs*
(Reply) (Thread)
[User Picture]
From:omnifarious
Date:January 26th, 2006 06:06 am (UTC)
(Link)

That's not the worst part actually. I added a bit to the post. Also, if you notice, all the memory management is being done by hand. There are functions that are chalk full of delete statements.

(Reply) (Parent) (Thread)
[User Picture]
From:scottscidmore
Date:January 26th, 2006 09:24 am (UTC)
(Link)
I've only worked with XML a little, but so far XML parsing has mostly consistent of calling ond XML reader or another, and then perhaps doing a little clean-up afterwards.

The example code looks like it was written by someone who coded at Microsoft waaay to long. If you need to tell what a bit of (XML) data means elsewhere in the system, it would seem to be better to use the literal string in the code and put the other in a comment (and name receiving variables meaningfully).


(Reply) (Parent) (Thread)
[User Picture]
From:omnifarious
Date:January 26th, 2006 02:07 pm (UTC)
(Link)

Exactly. The chances (if you use the same element name or attribute name for something) of wanting to globally change all places where one name is used to another is kind of ridiculous.

In this case, I believe there is a complication involving the fact that the host character set is EBCDIC. I think (but don't know yet) that's why all the transcoding is happening. But even with that, you can either do the transcoding on the fly, or generate a big map at startup that maps one set of strings to their transcoded versions instead of using this stupid naming scheme and all the tedious, error-prone code.

What impresses me most about all this is the fact the program hardly ever crashes, despite the fact that many modifications require an insane amount of attention to detail.

(Reply) (Parent) (Thread)
[User Picture]
From:elkman
Date:January 26th, 2006 05:05 pm (UTC)
(Link)
Why is "VSAM" in there twice? Wasn't the first "VSAM" enough to identify that it was trying to request fields from a VSAM file?
(Reply) (Thread)
[User Picture]
From:omnifarious
Date:January 26th, 2006 05:20 pm (UTC)
(Link)

Well, the first is to identify the subsystem (SYSTEM_VSAM) the #defines are for. No namespaces for those things, and the code doesn't use namespaces for anything else anyway. Then the next is to identify exactly what the constant represents.

The code actually has subsystems that are non-mainframe ones. The same (more or less) codebase is compiled on 5 different platforms.

But you're right, the name is stupidly verbose and largely useless anyway. It does more to obscure than to help.

(Reply) (Parent) (Thread)