Author |
Topic |
|
Nels_P_Olsen
Senior Member
USA
47 Posts |
Posted - Jan 07 2004 : 1:46:49 PM
|
We're trying to steer our boss towards a coding style where the code is actually readable and readily intelligible by someone other than its author. This involves not abbreviating stuff, using nouns/verbs/adjectives in the appropriate context, and ditching Hungarian notation. (The de facto Hungarian notation currently used in our code is a carryover from PowerBuilder, with strange additions, resulting in many misnomers, and is inconsistently applied, making a mess ...)
The biggest barrier at this point is the objection that dropping Hungarian notation prevents us from controlling the sorting in Intellisense member listboxes. My boss wants to see all "his" stuff separate from System/Microsoft stuff, and also wants to see his stuff subsorted by member type (field, property, method, event, etc). He says the listboxes are intolerably large and jumbled unless he uses his prefix scheme.
His approach still doesn't deal well with overriding or implementing a member defined in the System/Microsoft namespace -- is it "theirs" or "ours"?
A nice feature to have would be grouping the members by the full type name in which they are implemented or last overriden. This would free us from the last hurdle in the battle against cryptic identifier names ... |
- Nels |
|
mrcode925
Ketchup Master
USA
54 Posts |
Posted - Jan 07 2004 : 2:41:05 PM
|
Ouch. Does your boss hitch his car to a post in the parking lot every morning? New tools = new way of doing things. I can't remember the last time I wanted to check the value of g_oAcct.m_dIntRt instead of just Account.InterestRate. How can he justify the old way as productive? Oh well, mine is but to wonder... yours is but to suffer. |
If it starts to make sense, you're in a cult. |
Edited by - mrcode925 on Jan 07 2004 2:41:44 PM |
|
|
LarryLeonard
Tomato Guru
USA
1041 Posts |
Posted - Jan 07 2004 : 3:27:46 PM
|
That's an unfair analogy. Can you spot the potential bug in this code?
if (0 == Account.InterestRate) ...
How about this code?
if (0 == Account.dInterestRate) ...
The "d" wart means that variable is a "double"... knowing at a glance that we're comparing a floating point number to zero (oops!) helps me catch this bug more easily. And I am four-square behind anything that does that.
|
Edited by - LarryLeonard on Jan 08 2004 08:37:35 AM |
|
|
Nels_P_Olsen
Senior Member
USA
47 Posts |
Posted - Jan 07 2004 : 3:31:03 PM
|
Well, if you're part-owner of the company, no justification is required |
- Nels |
|
|
Stephen
Tomato Guru
United Kingdom
781 Posts |
Posted - Jan 08 2004 : 04:35:00 AM
|
The real bug is that you're assigning, not comparing. |
Stephen Turner ClickTracks http://www.clicktracks.com/ Winner: ClickZ's Best Web Analytics Tool 2003 & 2004
|
|
|
LarryLeonard
Tomato Guru
USA
1041 Posts |
Posted - Jan 08 2004 : 08:39:30 AM
|
The funny part is, in real code, I always put the non-lvalue on the left just to avoid that bug... I've fixed it now...
|
|
|
mrcode925
Ketchup Master
USA
54 Posts |
Posted - Jan 08 2004 : 09:16:26 AM
|
quote: Originally posted by LarryLeonard
That's an unfair analogy. Can you spot the potential bug in this code?
if (0 == Account.InterestRate) ...
If I am not mistaken, that bug is due to assumptions about the precision of doubles and does not have anything to do with comparing and int to a double. The int will harmlessly be promoted to a double for the comparison. You have the same problem comparing one double to another. This is a more fair analogy.
Account.InterestRate = ".029";
You will get a compiler error here. What do we do about it? Easy. Place the insertion caret on InterestRate and voila! VAX tells us that InterestDate is a double. If you are lazy, just hover over it and a tooltip will tell you the same thing.
If you really want hard work, you can read the compiler error which says that there is no type conversion for character array/string to double.
VAX goes one step further and shows yout that InterestRate is a double when you are writing the code to begin with so the error could have been caught during original development before Ctrl+Atl+B.
Alright, I am going a bit overboard here, but it is a little tongue-in-cheek. The point is that there are new tools now. If we were delevoping software in Notepad or the ancient Developer's Workbench for DOS then hungarian is a sensible option. Today, it is not only overkill, it has the potential to become confusing.
...
// 07-29-1998 header file for Account class
// this file is included everywhere
// an account object is used and
// members are read directly for speed
protected:
double m_fInterestRate; // 05-14-2001 Increased precision
// to double for new calculations
...
Oops! See the potential for bugs here? |
If it starts to make sense, you're in a cult. |
|
|
Stephen
Tomato Guru
United Kingdom
781 Posts |
Posted - Jan 08 2004 : 09:46:59 AM
|
You know, there are both advantages and disadvantages to Hungarian notation. We can probably all list several factors on each side. Different developers rate different factors higher or lower, and thus reach different conclusions about whether Hungarian notation is a good or a bad thing overall. De gustibus non disputandum est.
(And that bit of philosophy is enough to promote me to Guru ). |
Stephen Turner ClickTracks http://www.clicktracks.com/ Winner: ClickZ's Best Web Analytics Tool 2003 & 2004
|
|
|
LarryLeonard
Tomato Guru
USA
1041 Posts |
Posted - Jan 08 2004 : 09:53:08 AM
|
Yeah, it was a bad example, but there's a very good reason for that: I was too lazy to come up with a better one. (Yours isn't any better, though - all I have to do is change m_fInterestRate to m_dInterestRate and let the compiler catch the changes for me, or use one of our wonderful modern search and replace tools). Unless there's already a m_dInterestRate - better check for that first.)
Regarding Hungarian, I agree with everything you've said... in a perfect world, full of code written only by you and me, or by programmers at least as smart as we.
But bitter, bitter (I mean bitter) experience has taught me that Hungarian notation is extrememly useful when you're reading code that appears to have been written by a batallion of malicious, retarded monkeys. Which, for me, sadly, is pretty much every day...
|
|
|
Uniwares
Tomato Guru
Portugal
2322 Posts |
Posted - Jan 08 2004 : 10:06:22 AM
|
quote: Originally posted by LarryLeonard
... code that appears to have been written by a batallion of malicious, retarded monkeys.
So you are the one working with my ex-colleagues. |
|
|
Nels_P_Olsen
Senior Member
USA
47 Posts |
Posted - Jan 08 2004 : 11:30:53 AM
|
All too often, the Hungarian notation I encounter is stuff like f_ prefixing methods, o_ preceding arguments (why o_ ?), i_ preceding instance variables, or l_ preceding locals. This doesn't help worth crap, it only makes the identifiers more clunky.
When prefixes do contain warts indicating the datatype, it often provides the excuse that a sufficient name has already been created by the prefix itself, so you end up with identifiers like lo or str1, str2, str3 ...
Here's a particularly horrid example of code I have to deal with, plucked out of an "elegantly" organized function 500 or more lines long. How has the intermittent use of Hungarian notation helped here?
(Notice how the author likes to put comments trailing individual statements, as if it were assembly language ... written like this, it might as well be )
if j > 0 then
// last one is screen's title...
ls_parms.string_[j + 1]="Select Enclosures"
ls_parms.boolean_[1]=TRUE // multi-selection....
openWithParm(w_get_strings,ls_parms)
ls_parms = Message.PowerObjectParm
if (ls_parms.boolean_[1]) then
tot_encls = UpperBound(ls_parms.string_)
else
tot_encls = 0 // interrupt
end if
if tot_encls > 0 then
// Populate enclosures...
lu_stack = create u_stack
for i = 1 to tot_encls
str1 = trim(ls_parms.string_[i])
k = len(str1)
for j = k - 2 to 1 step -1
// We need only last part enclosed in parenteses...
ch = mid(str1,j,1)
if ch = "(" then
str2 = mid(str1, j + 1, (k - j - 1))
// d_enclosure has fields: encl, qty, recordid, act_key, numb_, desc.
// Any change should be reflected here as well...
str3 = str2 + "~t" + "1" + "~t" + "~t" + string(o_act_key) + "~t"
lu_stack.uf_pipe_in(str3)
// insert into gmin_mncl (act_key, enclosure,qty,numb_) values (:o_act_key, :str2, 1, :i) USING i_SQLCA;
EXIT
end if
next
next
end if
end if
|
- Nels |
|
|
LarryLeonard
Tomato Guru
USA
1041 Posts |
Posted - Jan 08 2004 : 11:49:54 AM
|
quote:
... stuff like f_ prefixing methods, o_ preceding arguments (why o_ ?), i_ preceding instance variables, or l_ preceding locals.
...you end up with identifiers like lo or str1, str2, str3 ...
... the author likes to put comments trailing individual statements...
Ewwwww .... That is horrid - no wonder you don't like Hungarian! And I love the use of i, j, and k as variables - that sure makes it easy to follow!
All I can say is, Hungarian can't save coding style like this, but abusing it can sure make it worse... Well, I wouldn't like ice cream, either, if someone covered it with poop and shoved it my ear... Good luck - I thought I had it bad!
|
|
|
|
Topic |
|
|
|