Sign in with
Sign up | Sign in
Your question

How NOT to code

Last response: in Video Games
Share
Anonymous
September 26, 2005 5:32:44 AM

Archived from groups: rec.games.roguelike.development (More info?)

I've just stumbled about this line of code from the Angband source while
browsing r.g.r.a:

/* Get the monster race */
r_ptr = &r_info[m_ptr->r_idx];

That's almost art; in a negative sense. And people say Angband has the
nicest source base of all major RLs - I guess I should thank god every day
that I've never bothered to look at the others *shudder*. I'm beginning to
understand why people were so hot about those newfangled OOP languages and
hate pointers so much. Probably because of people who wrote code like that.
I mean in Java/C#/etc all the standard classes have LongReadableNames which
automatically encourages programmers to follow that style. Whatever I'll
continue to use C because there's nothing that stops you from just doing
"monster->race" instead except if you really need to support that pre-ANSI
compiler on your 286SX.

Let's look at the abomination one last time:

r_ptr = &r_info[m_ptr->r_idx];

and remember kids: don't try this at home or in your CVS repository.

More about : code

Anonymous
September 26, 2005 5:32:45 AM

Archived from groups: rec.games.roguelike.development (More info?)

Oh well,

I dont find it that bad personally.
Go check http://thedailywtf.com/ if you think this is bad.

By the way, monster->race is just as silly for readability , what
would the type be of race ? String, integer, struct ?
At least with monster->r_idx you know it's an index ( int ).

T.
Anonymous
September 26, 2005 6:01:28 AM

Archived from groups: rec.games.roguelike.development (More info?)

copx wrote:
> I guess I should thank god every day
> that I've never bothered to look at the others *shudder*.

I like Crawl's source. It's quite incredible. I think that
example was not that bad, it was actually readable..
Related resources
Anonymous
September 26, 2005 7:32:02 AM

Archived from groups: rec.games.roguelike.development (More info?)

konijn_ wrote in news:1127703784.892298.170770
@g47g2000cwa.googlegroups.com:

> By the way, monster->race is just as silly for readability , what
> would the type be of race ? String, integer, struct ?
> At least with monster->r_idx you know it's an index ( int ).

'race' is a race, no doubt. You know, like human, dwarf, elf, liche, etc.
The name tells you what it means, which is all you need if you are trying
to understand the code.

Why should we care what the representation datatype is? If you knew that
it was a struct, for example, what good would that do? If you are
planning on maintaining the code, you'd still have to know the proper
conventions for using that struct which you can only get through reading
the definition and the comments that go with it. If you want to use
monster->race properly, you'll have to read documentation; no prefix in
the world can save you from that.

I've never understood the motivation for prefix notations. Can someone
put some light on this issue?
Anonymous
September 26, 2005 10:43:36 AM

Archived from groups: rec.games.roguelike.development (More info?)

>Suboptimal"? RACE * and r_idx are probably of the same size.

You should check that statement and come back to us,
if you dare.

T.
Anonymous
September 26, 2005 12:15:01 PM

Archived from groups: rec.games.roguelike.development (More info?)

Martin Read wrote:
> "copx" <invalid@invalid.com> wrote:
> >RACE * and r_idx are probably of the same size.
>
> Tell that to the guys running x86-64 systems, where int is 32 bits but
> void * is 64 bits.

It's not going be too many years before the int will be 64 bits on
those systems as well.

Having the pointer size be different than the index size is a very
unstable configuration.
--
Jeff Lait
(POWDER: http://www.zincland.com/powder)
Anonymous
September 26, 2005 12:22:42 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 03:32:02 GMT,
Brendan Guild wrote:

> konijn_ wrote in news:1127703784.892298.170770
> @g47g2000cwa.googlegroups.com:
>
>> By the way, monster->race is just as silly for readability , what
>> would the type be of race ? String, integer, struct ?
>> At least with monster->r_idx you know it's an index ( int ).
>
> 'race' is a race, no doubt. You know, like human, dwarf, elf, liche, etc.
> The name tells you what it means, which is all you need if you are trying
> to understand the code.
>
> Why should we care what the representation datatype is? If you knew that
> it was a struct, for example, what good would that do? If you are
> planning on maintaining the code, you'd still have to know the proper
> conventions for using that struct which you can only get through reading
> the definition and the comments that go with it. If you want to use
> monster->race properly, you'll have to read documentation; no prefix in
> the world can save you from that.
>
> I've never understood the motivation for prefix notations. Can someone
> put some light on this issue?

Well, it becomes much harder when you're using structs, pointers to
structs, indices and small green plastic turtles interchangeably.

It becomes even worse when you've got pointers to structs, pointers
to pointers to structs and pointers to pointers to pointers to turtles,
all casted to our favorite void*.

Or when both your indices and unique hashes are ints.

--
Radomir `The Sheep' Dopieralski @**@_
<..> ] 0110110?
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 12:26:05 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 01:32:44 +0200,
copx wrote:

> /* Get the monster race */
> r_ptr = &r_info[m_ptr->r_idx];

I don't get what you mean. Maybe the fact that it's monster race
is not stated clearly enough? Would you rather prefer:

/* monster race monster race monster race monster race */
race_ptr /* monster race */ = &race_info /* mosnter race */ [monster_ptr
/* monster */ -> race_idx /* monster race */]; /* Yes! it's about monster
race! */
/* monster race monster race monster race monster race */

Is it more readable to you? :p 
--
Radomir `The Sheep' Dopieralski @**@_
(^^) 3 Bee!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 12:35:55 PM

Archived from groups: rec.games.roguelike.development (More info?)

Radomir 'The Sheep' Dopieralski wrote:
> At Mon, 26 Sep 2005 13:46:50 +0200,
> copx wrote:
>
> > "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
> > news:1127703784.892298.170770@g47g2000cwa.googlegroups.com...
> >> Oh well,
> >>
> >> I dont find it that bad personally.
> >> Go check http://thedailywtf.com/ if you think this is bad.
> >>
> >> By the way, monster->race is just as silly for readability , what
> >> would the type be of race ? String, integer, struct ?
> >> At least with monster->r_idx you know it's an index ( int ).
> >
> > The whole point of that call was to get a monster race pointer and you don't
> > know what type of data that pointer points to anyway. Knowing the type of
> > r_idx is not an accomplishment because if you use monster->race you don't
> > even need that variable. You get the same result (retriving the pointer to
> > the monster's race) without having to use three memory address operators
> > ([],->, &). Without the comment or good knowledge of the source base the
> > r_ptr = ... line is complety unreadable!
>
> But the comment *is* there and it's part of the code.

The comment is the proof that the code isn't good. What copx is
railing against is the sort of coding that requires inane comments like
"Gets the monster's race". This is what throws the claim that "r for
race is a good convention" out the window. If there was a standard r
for race, there should have been no need for that comment because the
reader would see:

race_ptr = &race_info[monster_ptr->race_idx];

The writer, obviously, didn't see that if they felt it was necessary to
add the comment.

My main problem with this code snippet is: "Where is r_info defined?"
I'd put my bets on a global variable, in which case it really should
have some form of prefix or suffix to identify it as such. It may be
that Angband ensures foo_info is always global. For those of you
writing in OOP languages, please consider the same issue with member
variables. It doesn't matter what you prefix them with, so long as one
can tell from a single snippet what scope the variables are coming
from.

The secondary problem is that if this occurs more than once, we are
being locked into the pattern of having indexable race data. Using an
index to access race data is a great idea. Exposing this fact to the
entire code base isn't. Uses of r_info should be wrapped by functions.
As has already been recommended:

R_PTR_TYPE
getRaceFromMonster(M_PTR_TYPE m_ptr)
{
return &r_info[m_ptr->r_idx];
}

The resulting code snippet becomes:

r_ptr = getRaceFromMonster(m_ptr);

The comment becomes entirely unnecessary. And, if you want to change
the way you store or define the actual race structures, you have only
one place to change.

Please do not take this critism to be an insult to the Angband
developers. I'd rather it be seen as my own view of how, in an ideal
world, such code would be written. I'm posting this in the hopes that
people writing net-new code may learn something. Feel free to call out
any egregarious coding on my part - the sources for You Only Live Once
are available. I reserve the right to be defensive and petty, however
:>
--
Jeff Lait
(POWDER: http://www.zincland.com/powder)
Anonymous
September 26, 2005 1:23:33 PM

Archived from groups: rec.games.roguelike.development (More info?)

Radomir 'The Sheep' Dopieralski wrote in
news:slrndjfc4t.2mt.thesheep@atos.wmid.amu.edu.pl:

> At Mon, 26 Sep 2005 01:32:44 +0200,
> copx wrote:
>
>> /* Get the monster race */
>> r_ptr = &r_info[m_ptr->r_idx];
>
> I don't get what you mean. Maybe the fact that it's monster race
> is not stated clearly enough? Would you rather prefer:
>
> /* monster race monster race monster race monster race */
> race_ptr /* monster race */ = &race_info /* mosnter race */
> [monster_ptr /* monster */ -> race_idx /* monster race */]; /* Yes!
> it's about monster race! */
> /* monster race monster race monster race monster race */

I imagine copx would have prefered something like:

Race *getMonsterRace(Monster *monster)
{
return &r_info[monster->race];
}
..
..
..
race = getMonsterRace(monster);

Readable names and abstraction are very nice; aren't they?
Anonymous
September 26, 2005 1:27:02 PM

Archived from groups: rec.games.roguelike.development (More info?)

Okay,

last reply, I promise ;) 

First off, you didnt look :

/* Read the monster race */
rd_s16b(&m_ptr->r_idx); /* Hope copx can handle looking at this ;)  */

Also, notice how much would be gained if we play on a 64 bit system,
and notice that this also saves 2k per savefile ( I will admit you'd be
hard pressed to find 1000 monsters on a level ).

The same technique is consistently used throughout the source to
identify item indexes, ego indexes and artefact indexes. That gets a
lot of K in the end.

Just admit that this code just pulled one of your triggers,
you know the ones that make you post angry blurbs ;) 

And that it is the result of a design that beats most roguelikes
on efficiency in speed and size.

T.
Anonymous
September 26, 2005 5:33:32 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjfc4t.2mt.thesheep@atos.wmid.amu.edu.pl...
> At Mon, 26 Sep 2005 01:32:44 +0200,
> copx wrote:
>
>> /* Get the monster race */
>> r_ptr = &r_info[m_ptr->r_idx];
>
> I don't get what you mean. Maybe the fact that it's monster race
> is not stated clearly enough? Would you rather prefer:

monster->race

Readable and logical.

copx
Anonymous
September 26, 2005 5:33:33 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 13:33:32 +0200,
copx wrote:

>
> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
> Newsbeitrag news:slrndjfc4t.2mt.thesheep@atos.wmid.amu.edu.pl...
>> At Mon, 26 Sep 2005 01:32:44 +0200,
>> copx wrote:
>>
>>> /* Get the monster race */
>>> r_ptr = &r_info[m_ptr->r_idx];
>>
>> I don't get what you mean. Maybe the fact that it's monster race
>> is not stated clearly enough? Would you rather prefer:
>
> monster->race
>
> Readable and logical.

Not really. It either forces you to use certain data structure, which
is in this situation suboptimal, or to overload the '->' operator (can
it be overloaded in C++?), which is likely to create even more problems
later.

On the other hand, the "r for race" and "m for monster" are used
consequently trough all the sources, are quick to write and read
and are explained in the comments. What more would you want?

Note that information bloat can be much worse than lack of information.

--
Radomir `The Sheep' Dopieralski @**@_
(*+) 3 Sparkle
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 5:46:50 PM

Archived from groups: rec.games.roguelike.development (More info?)

"konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
news:1127703784.892298.170770@g47g2000cwa.googlegroups.com...
> Oh well,
>
> I dont find it that bad personally.
> Go check http://thedailywtf.com/ if you think this is bad.
>
> By the way, monster->race is just as silly for readability , what
> would the type be of race ? String, integer, struct ?
> At least with monster->r_idx you know it's an index ( int ).

The whole point of that call was to get a monster race pointer and you don't
know what type of data that pointer points to anyway. Knowing the type of
r_idx is not an accomplishment because if you use monster->race you don't
even need that variable. You get the same result (retriving the pointer to
the monster's race) without having to use three memory address operators
([],->, &). Without the comment or good knowledge of the source base the
r_ptr = ... line is complety unreadable!
As far as TDWTF is concerned I know that were is even worse code but that
doesn't make this code any better.
Of course I knew that there would be some people who whould try to defend it
but really it's undefendable. Just like the UNIX FHS..


copx
Anonymous
September 26, 2005 5:46:51 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 13:46:50 +0200,
copx wrote:

> "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
> news:1127703784.892298.170770@g47g2000cwa.googlegroups.com...
>> Oh well,
>>
>> I dont find it that bad personally.
>> Go check http://thedailywtf.com/ if you think this is bad.
>>
>> By the way, monster->race is just as silly for readability , what
>> would the type be of race ? String, integer, struct ?
>> At least with monster->r_idx you know it's an index ( int ).
>
> The whole point of that call was to get a monster race pointer and you don't
> know what type of data that pointer points to anyway. Knowing the type of
> r_idx is not an accomplishment because if you use monster->race you don't
> even need that variable. You get the same result (retriving the pointer to
> the monster's race) without having to use three memory address operators
> ([],->, &). Without the comment or good knowledge of the source base the
> r_ptr = ... line is complety unreadable!

But the comment *is* there and it's part of the code.

> As far as TDWTF is concerned I know that were is even worse code but that
> doesn't make this code any better.
> Of course I knew that there would be some people who whould try to defend it
> but really it's undefendable. Just like the UNIX FHS..

Go flame somewhere else.

--
Radomir `The Sheep' Dopieralski @**@_
(Uu) 3 Sigh!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 6:05:06 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjfnnm.rl6.thesheep@atos.wmid.amu.edu.pl...
> At Mon, 26 Sep 2005 13:33:32 +0200,
> copx wrote:
>
>>
>> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
>> Newsbeitrag news:slrndjfc4t.2mt.thesheep@atos.wmid.amu.edu.pl...
>>> At Mon, 26 Sep 2005 01:32:44 +0200,
>>> copx wrote:
>>>
>>>> /* Get the monster race */
>>>> r_ptr = &r_info[m_ptr->r_idx];
>>>
>>> I don't get what you mean. Maybe the fact that it's monster race
>>> is not stated clearly enough? Would you rather prefer:
>>
>> monster->race
>>
>> Readable and logical.
>
> Not really. It either forces you to use certain data structure, which
> is in this situation suboptimal, or to overload the '->' operator (can
> it be overloaded in C++?), which is likely to create even more problems
> later.

"Suboptimal"? RACE * and r_idx are probably of the same size. The race data
seems to be saved in the r_info array. How is using a direct pointer to the
related entry any less "optimal" than using an index?
The only thing I can think of is saving and restoring. But doing a little
extra work there is worth it because it keeps such glibberish out of the
rest of the source.

> On the other hand, the "r for race" and "m for monster" are used
> consequently trough all the sources, are quick to write and read
> and are explained in the comments. What more would you want?
>
> Note that information bloat can be much worse than lack of information.

Only in very extreme cases. The comment in this case would be completely
unecessary if the code would be readable to begin with. ASCII characters are
not a scarce resource and the time it take to type the source is an
irrelevant factor in programming. But if you have such a strong desire to
keep it short: monster->race requires a lot less characters than r_ptr =
&r_info[m_ptr->r_idx] + comment..
And abbreviations are always evil. They obscure the source (especially for
the casual reader) without providing any real gains. And you sound like
using them consequently would be some kind of accomplishment. If you don't
do that you might just as well use a real cryptography program on the
source...


copx
Anonymous
September 26, 2005 7:34:44 PM

Archived from groups: rec.games.roguelike.development (More info?)

"copx" <invalid@invalid.com> wrote:
>RACE * and r_idx are probably of the same size.

Tell that to the guys running x86-64 systems, where int is 32 bits but
void * is 64 bits.
--
Martin Read - my opinions are my own. share them if you wish.
\_\/_/ http://www.chiark.greenend.org.uk/~mpread/dungeonbash/
\ / "tempted white eyes blinded by the night hollow like the towers from the
\/ inside laura's a machine she's burning insane" fields of the nephilim
Anonymous
September 26, 2005 7:59:11 PM

Archived from groups: rec.games.roguelike.development (More info?)

At 26 Sep 2005 08:35:55 -0700,
Jeff Lait wrote:

> Radomir 'The Sheep' Dopieralski wrote:
>> At Mon, 26 Sep 2005 13:46:50 +0200,
>> copx wrote:
>> > "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
>> > news:1127703784.892298.170770@g47g2000cwa.googlegroups.com...

> The secondary problem is that if this occurs more than once, we are
> being locked into the pattern of having indexable race data. Using an
> index to access race data is a great idea. Exposing this fact to the
> entire code base isn't. Uses of r_info should be wrapped by functions.

We don't know where this snippet comes from -- the comments were only
about it's overall ugliness. :) 

Wrapping the uses of global variables by functions turns it practically
into an object, doesn't it?

--
Radomir `The Sheep' Dopieralski @**@_
(TT) 3 Waaah!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 8:19:33 PM

Archived from groups: rec.games.roguelike.development (More info?)

"konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
news:1127742216.309272.324740@g47g2000cwa.googlegroups.com...
> >Suboptimal"? RACE * and r_idx are probably of the same size.
>
> You should check that statement and come back to us,
> if you dare.

What do you mean? If r_idx is an int it's 4 byte on a x86 32bit machine. A
pointer is 4 byte in that setup, too..

copx
Anonymous
September 26, 2005 8:19:34 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 16:19:33 +0200,
copx wrote:

>
> "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
> news:1127742216.309272.324740@g47g2000cwa.googlegroups.com...
>> >Suboptimal"? RACE * and r_idx are probably of the same size.
>>
>> You should check that statement and come back to us,
>> if you dare.
>
> What do you mean? If r_idx is an int it's 4 byte on a x86 32bit machine. A
> pointer is 4 byte in that setup, too..

So there's no difference on *your* box.
Say again, what platforms is Angband available for?

--
Radomir `The Sheep' Dopieralski @**@_
(Xx) 3 ...
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 8:43:53 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjg1es.dgs.thesheep@atos.wmid.amu.edu.pl...
> At Mon, 26 Sep 2005 16:19:33 +0200,
> copx wrote:
>
>>
>> "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
>> news:1127742216.309272.324740@g47g2000cwa.googlegroups.com...
>>> >Suboptimal"? RACE * and r_idx are probably of the same size.
>>>
>>> You should check that statement and come back to us,
>>> if you dare.
>>
>> What do you mean? If r_idx is an int it's 4 byte on a x86 32bit machine.
>> A
>> pointer is 4 byte in that setup, too..
>
> So there's no difference on *your* box.
> Say again, what platforms is Angband available for?

x86 32bit and a bunch of others that don't matter at all. But even if they
would we are talking about only a few bytes of maximal difference here. Lets
say you keep 1000 monsters in mem. at a time (that's probably the most any
normal RL will ever do) and the difference between the numeric value and the
pointer is 2 bytes.
That means you're obscuring the entire source base and slow down access to
the race pointer everywhere for saving 2KB of RAM! Even if you care about
nothing except efficiency that's just insane.


copx
Anonymous
September 26, 2005 8:43:54 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 16:43:53 +0200,
copx wrote:

> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
> Newsbeitrag news:slrndjg1es.dgs.thesheep@atos.wmid.amu.edu.pl...
>> At Mon, 26 Sep 2005 16:19:33 +0200,
>> copx wrote:
>>> "konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
>>> news:1127742216.309272.324740@g47g2000cwa.googlegroups.com...
>>>>> Suboptimal"? RACE * and r_idx are probably of the same size.
>>>> You should check that statement and come back to us,
>>>> if you dare.
>>> What do you mean? If r_idx is an int it's 4 byte on a x86 32bit machine.
>>> A pointer is 4 byte in that setup, too..
>> So there's no difference on *your* box.
>> Say again, what platforms is Angband available for?

> x86 32bit and a bunch of others that don't matter at all.

So you're basically telling us that everyone should code the way *you*
like it, because everything else doesn't matter at all? ;) 
Cool it.

> But even if they
> would we are talking about only a few bytes of maximal difference here. Lets
> say you keep 1000 monsters in mem. at a time (that's probably the most any
> normal RL will ever do) and the difference between the numeric value and the
> pointer is 2 bytes.
> That means you're obscuring the entire source base and slow down access to
> the race pointer everywhere for saving 2KB of RAM! Even if you care about
> nothing except efficiency that's just insane.

I won't say anything about your statements that the size of source code
doesn't have any impact on it's readability and maintainability, not to
mention the time needed to just type it in.

Also note that you are the only one who thinks it's obscure. It's
perfectly clear code for any moderately experienced C programmer.

Sure, if it was C++, Java, C#, or even a project in C but started with
different coding standards and schemas -- then your could use
'monster->race' or 'monster[race]' or even 'race[monster]'. But in the
Angband source it'd stick out like a sore thumb.

There's no such thing as "good style for everyone". Ever heard about
'consistency'?

--
Radomir `The Sheep' Dopieralski @**@_
(><) 3 Ouch!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 8:43:55 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 17:26:32 +0200,
copx wrote:

>> Sure, if it was C++, Java, C#, or even a project in C but started with
>> different coding standards and schemas -- then your could use
>> 'monster->race' or 'monster[race]' or even 'race[monster]'. But in the
>> Angband source it'd stick out like a sore thumb.
>
> I actually agree with you here. If you would only change that part of the
> Angband source it would certainly destroy the consistency. But my point was
> that the present consistency is a consistency of terribly low-level, hard to
> read code. I don't want to say that it didn't make sense back then Angband
> was still running on a 286 and C compilers completely ignored all identifier
> characters after the sixth and completely rewriting it today wouldn't make
> much sense either. I just wanted to urge developers not to write new code in
> that style nowadays.
> Of course most of them won't just because most professional programmers use
> modern OOP languages anyway and there the language naturally leads to either
> monster.race, monster.getRace() or something similar. Even a industry run by
> geeks had to realize that the coding style critized by me was a goddamn
> nightmare.

So what's the fuss about?

--
Radomir `The Sheep' Dopieralski @**@_
(><) 3 Ouch!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 8:51:03 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Martin Read" <mpread@chiark.greenend.org.uk> schrieb im Newsbeitrag
news:8Xs*bhGZq@news.chiark.greenend.org.uk...
> "copx" <invalid@invalid.com> wrote:
>>RACE * and r_idx are probably of the same size.
>
> Tell that to the guys running x86-64 systems, where int is 32 bits but
> void * is 64 bits.

A difference of 4bytes and we all know that x86-64 systems are really
RAM-starved. Come on that's 4KB for 1000 monsters on machines that usually
have above >= 1GB of RAM! And don't forget that you're sacrificing
performance for this "optimization".

copx
Anonymous
September 26, 2005 8:51:04 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 16:51:03 +0200,
copx wrote:

>
> "Martin Read" <mpread@chiark.greenend.org.uk> schrieb im Newsbeitrag
> news:8Xs*bhGZq@news.chiark.greenend.org.uk...
>> "copx" <invalid@invalid.com> wrote:
>>>RACE * and r_idx are probably of the same size.
>>
>> Tell that to the guys running x86-64 systems, where int is 32 bits but
>> void * is 64 bits.
>
> A difference of 4bytes and we all know that x86-64 systems are really
> RAM-starved. Come on that's 4KB for 1000 monsters on machines that usually
> have above >= 1GB of RAM! And don't forget that you're sacrificing
> performance for this "optimization".

The 64bit sparc stations 10 sitting in the room next door have indeed problems
with the amount of ram they have, especially with their large 64bit RISC
binaries. Even when systems have lots of RAM, there are usually multiple
users using it, not to mention that every one of them runs usually
multiple programs.

The world doesn't consist solely of the windows box sitting on yor desktop.

--
Radomir `The Sheep' Dopieralski @**@_
($s) 3 Ching!
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 9:23:36 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Jeff Lait" <torespondisfutile@hotmail.com> wrote:
>Martin Read wrote:
>> Tell that to the guys running x86-64 systems, where int is 32 bits but
>> void * is 64 bits.
>
>It's not going be too many years before the int will be 64 bits on
>those systems as well.

A cursory google suggests that int was 32 bits on Alpha (which was
64-bit from the start) throughout that architecture's existence.
--
Martin Read - my opinions are my own. share them if you wish.
\_\/_/ http://www.chiark.greenend.org.uk/~mpread/dungeonbash/
\ / "tempted white eyes blinded by the night hollow like the towers from the
\/ inside laura's a machine she's burning insane" fields of the nephilim
Anonymous
September 26, 2005 9:29:57 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjg3et.oj4.thesheep@atos.wmid.amu.edu.pl...
> At Mon, 26 Sep 2005 16:51:03 +0200,
> copx wrote:
>
>>
>> "Martin Read" <mpread@chiark.greenend.org.uk> schrieb im Newsbeitrag
>> news:8Xs*bhGZq@news.chiark.greenend.org.uk...
>>> "copx" <invalid@invalid.com> wrote:
>>>>RACE * and r_idx are probably of the same size.
>>>
>>> Tell that to the guys running x86-64 systems, where int is 32 bits but
>>> void * is 64 bits.
>>
>> A difference of 4bytes and we all know that x86-64 systems are really
>> RAM-starved. Come on that's 4KB for 1000 monsters on machines that
>> usually
>> have above >= 1GB of RAM! And don't forget that you're sacrificing
>> performance for this "optimization".
>
> The 64bit sparc stations 10 sitting in the room next door have indeed
> problems
> with the amount of ram they have, especially with their large 64bit RISC
> binaries. Even when systems have lots of RAM, there are usually multiple
> users using it, not to mention that every one of them runs usually
> multiple programs.
>
> The world doesn't consist solely of the windows box sitting on yor
> desktop.

Come on, supporting old Sparc stations that are used by multiple users at
the same time certainly isn't a requirement for game.
If we would make that our collective goal someone should tell all the
Python, Ruby, Java, C# etc. coders that they are on the wrong trek...
Anonymous
September 26, 2005 9:38:53 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjg534.7ia.thesheep@atos.wmid.amu.edu.pl...
[snip]
>> I actually agree with you here. If you would only change that part of the
>> Angband source it would certainly destroy the consistency. But my point
>> was
>> that the present consistency is a consistency of terribly low-level, hard
>> to
>> read code. I don't want to say that it didn't make sense back then
>> Angband
>> was still running on a 286 and C compilers completely ignored all
>> identifier
>> characters after the sixth and completely rewriting it today wouldn't
>> make
>> much sense either. I just wanted to urge developers not to write new code
>> in
>> that style nowadays.
>> Of course most of them won't just because most professional programmers
>> use
>> modern OOP languages anyway and there the language naturally leads to
>> either
>> monster.race, monster.getRace() or something similar. Even a industry run
>> by
>> geeks had to realize that the coding style critized by me was a goddamn
>> nightmare.
>
> So what's the fuss about?

Maybe I should repeat a sentence from the post you've just replied too:

"I just wanted to urge developers not to write new code in that style
nowadays."

That's the point. What's unclear about that?


copx
Anonymous
September 26, 2005 9:38:54 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Mon, 26 Sep 2005 17:38:53 +0200,
copx wrote:

>
> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
> Newsbeitrag news:slrndjg534.7ia.thesheep@atos.wmid.amu.edu.pl...
> [snip]
>>> I actually agree with you here. If you would only change that part of the
>>> Angband source it would certainly destroy the consistency. But my point
>>> was
>>> that the present consistency is a consistency of terribly low-level, hard
>>> to
>>> read code. I don't want to say that it didn't make sense back then
>>> Angband
>>> was still running on a 286 and C compilers completely ignored all
>>> identifier
>>> characters after the sixth and completely rewriting it today wouldn't
>>> make
>>> much sense either. I just wanted to urge developers not to write new code
>>> in
>>> that style nowadays.
>>> Of course most of them won't just because most professional programmers
>>> use
>>> modern OOP languages anyway and there the language naturally leads to
>>> either
>>> monster.race, monster.getRace() or something similar. Even a industry run
>>> by
>>> geeks had to realize that the coding style critized by me was a goddamn
>>> nightmare.
>> So what's the fuss about?
> Maybe I should repeat a sentence from the post you've just replied too:
> "I just wanted to urge developers not to write new code in that style
> nowadays."
> That's the point. What's unclear about that?
The second paragraph, the one starting with "of course".
I can't still see the purpose of this thread ;) 

--
Radomir `The Sheep' Dopieralski @**@_
(==) 3 Yawn?
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 26, 2005 9:41:19 PM

Archived from groups: rec.games.roguelike.development (More info?)

copx a écrit :
> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
> Newsbeitrag news:slrndjg3et.oj4.thesheep@atos.wmid.amu.edu.pl...
>
>>At Mon, 26 Sep 2005 16:51:03 +0200,
>>copx wrote:
>>
>>
>>>"Martin Read" <mpread@chiark.greenend.org.uk> schrieb im Newsbeitrag
>>>news:8Xs*bhGZq@news.chiark.greenend.org.uk...
>>>
>>>>"copx" <invalid@invalid.com> wrote:
>>>>
>>>>>RACE * and r_idx are probably of the same size.
>>>>
>>>>Tell that to the guys running x86-64 systems, where int is 32 bits but
>>>>void * is 64 bits.
>>>
>>>A difference of 4bytes and we all know that x86-64 systems are really
>>>RAM-starved. Come on that's 4KB for 1000 monsters on machines that
>>>usually
>>>have above >= 1GB of RAM! And don't forget that you're sacrificing
>>>performance for this "optimization".
>>
>>The 64bit sparc stations 10 sitting in the room next door have indeed
>>problems
>>with the amount of ram they have, especially with their large 64bit RISC
>>binaries. Even when systems have lots of RAM, there are usually multiple
>>users using it, not to mention that every one of them runs usually
>>multiple programs.
>>
>>The world doesn't consist solely of the windows box sitting on yor
>>desktop.
>
>
> Come on, supporting old Sparc stations that are used by multiple users at
> the same time certainly isn't a requirement for game.
> If we would make that our collective goal someone should tell all the
> Python, Ruby, Java, C# etc. coders that they are on the wrong trek...

Angband is old code. For today standards it might be unacceptable to
write it like that but for the time it has been writen it was probably
common practice. And at the time, compatibility with Sun Sparc might
have been considered important too. Now to "fix" that situation would
require a major code rewrite.
Anonymous
September 26, 2005 10:33:51 PM

Archived from groups: rec.games.roguelike.development (More info?)

"konijn_" <konijn@gmail.com> schrieb im Newsbeitrag
news:1127752022.569913.246960@g47g2000cwa.googlegroups.com...
> Okay,
>
> last reply, I promise ;) 

Ok, to avoid an endless, fruitless discussion that would only take valueable
time away from our roguelike projects I won't continue this argument. I've
made my point clear enough anyway so there's no reason.
Anonymous
September 26, 2005 11:38:08 PM

Archived from groups: rec.games.roguelike.development (More info?)

On 2005-09-26, Brendan Guild <dont@spam.me> wrote:
> I've never understood the motivation for prefix notations. Can someone
> put some light on this issue?

Joel Spolsky has an article about how to use prefix notation or
Hungarian notation: http://www.joelonsoftware.com/articles/Wrong.html

Basically, you don't use prefixes to mark type, you use them to mark
semantic information about a variable. In Joel's example, the prefix
"us" (for "unsafe") is used for all string variables that may contain
input from an user in their web application, because you must never
print those strings on a web page without first removing HTML
formatting. You can't use the language's type system because safe and
unsafe strings have the same type. The difference is semantical, safe
strings are ones that can be assumed not to contain malicious HTML code
while unsafe strings might contain bad things.

Using prefix notation for variable type doesn't make much sense in any
programming language with a proper type system, but using prefixes for
semantic properties that can't easily be expressed by the type system
might still be a good idea.

--
Risto Saarelma
Anonymous
September 27, 2005 12:20:38 AM

Archived from groups: rec.games.roguelike.development (More info?)

Risto Saarelma wrote in news:D h9in0$dfp$1@nyytiset.pp.htv.fi:

> Basically, you don't use prefixes to mark type, you use them to mark
> semantic information about a variable. In Joel's example, the prefix
> "us" (for "unsafe") is used for all string variables that may contain
> input from an user in their web application, because you must never
> print those strings on a web page without first removing HTML
> formatting. You can't use the language's type system because safe and
> unsafe strings have the same type. The difference is semantical, safe
> strings are ones that can be assumed not to contain malicious HTML
> code while unsafe strings might contain bad things.
>
> Using prefix notation for variable type doesn't make much sense in
> any programming language with a proper type system, but using
> prefixes for semantic properties that can't easily be expressed by
> the type system might still be a good idea.

I agree completely. The name of a variable should indicate its role in
the program as abstractly as possible. Safe or Unsafe is not part of
the type, it is part of the role, as is scope related information. The
things that we would call 'type' information, especially in C, are not
things that we should be forcing ourselves to remember. You don't need
to know a variable's type to pass it into a function, but you do need
to know what it represents.

For example: getMonsterRace(x). We don't need to know that a monster is
represented as an index, a pointer to a struct, a struct, a string, or
even an array, just so long as getMonsterRace accepts a monster, and
our variable holds a monster when we pass it in.
Anonymous
September 28, 2005 9:21:56 PM

Archived from groups: rec.games.roguelike.development (More info?)

Ha. Look at all the people who just jumped in defending the said
crime-against-humanity-in-code-form. The real problem is that too many
people accept this as being the status quo rather than trying something
better.

Just my 2c.
Anonymous
September 29, 2005 2:26:03 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Shedletsky" <mylastname@stanford.edu> wrote:
>Ha. Look at all the people who just jumped in defending the said
>crime-against-humanity-in-code-form.

I dispute the severity of its awfulness. The code is correct and
commented, so a (quite modest) lack of clarity is tolerable.
--
Martin Read - my opinions are my own. share them if you wish.
\_\/_/ http://www.chiark.greenend.org.uk/~mpread/dungeonbash/
\ / "tempted white eyes blinded by the night hollow like the towers from the
\/ inside laura's a machine she's burning insane" fields of the nephilim
Anonymous
September 29, 2005 8:50:54 PM

Archived from groups: rec.games.roguelike.development (More info?)

copx wrote:
> I've just stumbled about this line of code from the Angband source while
> browsing r.g.r.a:
>
> /* Get the monster race */
> r_ptr = &r_info[m_ptr->r_idx];

Hmmm. It reads "set the pointer r_ptr to the absolute address of
the monster m_ptr's r_index'th element of the r_info array."

Just from the snippet, it becomes clear that m_ptr is a pointer
to a monster record type, and r_idx is an integer member of that
record type which gives an index into the race table for the
monster's race.

That's perfectly readable for people who code in C, although I'd
have used some parens in the code just to make the argument of
the & operation clearer.


> That's almost art; in a negative sense. And people say Angband has the
> nicest source base of all major RLs - I guess I should thank god every day
> that I've never bothered to look at the others *shudder*.


I'm just going to go out on a limb here and guess you were
never really comfortable with pointers? :-) Because, dude,
an expression with multiple pointer operations is really no
more baffling than an expression with multiple number
operations. Seriously, would you consider

x = a+b*(c/d);

to be a crime against humanity? 'Cause it's the same
complexity if you actually "get" pointers, right down to
the superfluous ambiguity resolved by operation precedence
rules since the parens for & were left out of the original
line.

Bear
Anonymous
September 29, 2005 11:53:52 PM

Archived from groups: rec.games.roguelike.development (More info?)

"Ray Dillinger" <bear@sonic.net> schrieb im Newsbeitrag
news:o XU_e.700$Aw.11161@typhoon.sonic.net...
[snip]

I've already said that I won't continue this discussion. Please read the
entire thread before you reply. Also my understanding of pointers is just
fine, thanks. My critic was targetted at the abbreviations and the
unnecessary complexity. And then there's the lack of information hiding and
decoupling which wasn't even discussed in this thread. Jeff pointed out the
other issue that the code uses a global variable which isn't even clearly
marked as such. The code is not good. You may disagree and that's fine with
me as I said before I don't want to continue this fruitless discussion. I
was wrong in starting a thread on an almost religious topic in the first
place. Such threads never get anythere but I didn't thought about that when
I posted the original message.
Please let it rest, ok?
Anonymous
September 29, 2005 11:53:53 PM

Archived from groups: rec.games.roguelike.development (More info?)

At Thu, 29 Sep 2005 19:53:52 +0200,
copx wrote:

>
> "Ray Dillinger" <bear@sonic.net> schrieb im Newsbeitrag
> news:o XU_e.700$Aw.11161@typhoon.sonic.net...
> [snip]
>
> I've already said that I won't continue this discussion. Please read the
> entire thread before you reply. Also my understanding of pointers is just
> fine, thanks. My critic was targetted at the abbreviations and the
> unnecessary complexity. And then there's the lack of information hiding and
> decoupling which wasn't even discussed in this thread. Jeff pointed out the
> other issue that the code uses a global variable which isn't even clearly
> marked as such. The code is not good. You may disagree and that's fine with
> me as I said before I don't want to continue this fruitless discussion. I
> was wrong in starting a thread on an almost religious topic in the first
> place. Such threads never get anythere but I didn't thought about that when
> I posted the original message.
> Please let it rest, ok?

If *you* don't want to continue the discussion, then *you* should just
stopping posting to this thread.

On the other hand, if you don't want *anybody* to continue this
discussion, then there are some troubles -- you see, it's usenet and
you can't really forbid anybody posting to this thread.

--
Radomir `The Sheep' Dopieralski @**@_
(==) 3 Yawn?
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 30, 2005 12:19:39 AM

Archived from groups: rec.games.roguelike.development (More info?)

"copx" <invalid@invalid.com> writes:

> Of course but Dillinger was personally attacking me suggesting that I don't
> understand how pointers work and I didn't want to let that stand unanswered.

I think we just found out where Neo went...

sherm--

--
Cocoa programming in Perl: http://camelbones.sourceforge.net
Hire me! My resume: http://www.dot-app.org
Anonymous
September 30, 2005 2:10:12 AM

Archived from groups: rec.games.roguelike.development (More info?)

"Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
Newsbeitrag news:slrndjoghe.4mr.thesheep@atos.wmid.amu.edu.pl...
[snip]
> If *you* don't want to continue the discussion, then *you* should just
> stopping posting to this thread.
>
> On the other hand, if you don't want *anybody* to continue this
> discussion, then there are some troubles -- you see, it's usenet and
> you can't really forbid anybody posting to this thread.

Of course but Dillinger was personally attacking me suggesting that I don't
understand how pointers work and I didn't want to let that stand unanswered.
However you're are right to say that the thread won't die any time soon as
long as I continue to reply to it. So I will ignore even such posts in
future. This is my last post in this thread take my word for it - there are
so many more important things to do.

I still don't want to see more C Roguelikes written like this, though. So I
suggest the following resources to any reader of the thread who is looking
for more information about why the originally posted code is bad and how to
write better code:

C - Elements of Style:
http://www.amazon.com/exec/obidos/tg/detail/-/155851291...

Code Complete:
http://www.amazon.com/exec/obidos/tg/detail/-/155615484...
Anonymous
September 30, 2005 2:10:13 AM

Archived from groups: rec.games.roguelike.development (More info?)

At Thu, 29 Sep 2005 22:10:12 +0200,
copx wrote:

>
> "Radomir 'The Sheep' Dopieralski" <thesheep@ sheep.prv.pl> schrieb im
> Newsbeitrag news:slrndjoghe.4mr.thesheep@atos.wmid.amu.edu.pl...
> [snip]
>> If *you* don't want to continue the discussion, then *you* should just
>> stopping posting to this thread.
>>
>> On the other hand, if you don't want *anybody* to continue this
>> discussion, then there are some troubles -- you see, it's usenet and
>> you can't really forbid anybody posting to this thread.
>
> Of course but Dillinger was personally attacking me suggesting that I don't
> understand how pointers work and I didn't want to let that stand unanswered.

I didn't get such an impression when reading the post. All I see is an
assumption (may be unfair), that it's pointer arithmetics that made you
think the code is complicated.

> However you're are right to say that the thread won't die any time soon as
> long as I continue to reply to it. So I will ignore even such posts in
> future. This is my last post in this thread take my word for it - there are
> so many more important things to do.

That the spirit! :)  Don't let some minor discussion to shake your nerves.

> I still don't want to see more C Roguelikes written like this, though.

Just don't look. People will write their hobby projects, and most probably
they'll learn as they go (since making a project that you don't learn
anything from is no fun). But that means that they will be pretty bad when
they begin.

> So I
> suggest the following resources to any reader of the thread who is looking
> for more information about why the originally posted code is bad and how to
> write better code:
>
> C - Elements of Style:
> http://www.amazon.com/exec/obidos/tg/detail/-/155851291...
>
> Code Complete:
> http://www.amazon.com/exec/obidos/tg/detail/-/155615484...

Thanks, been there, red those. But somehow I don't automatically believe
everything I read ;) 

--
Radomir `The Sheep' Dopieralski @**@_
(--) 3 ..zzZZ
. . . ..v.vVvVVvVvv.v.. .
Anonymous
September 30, 2005 4:16:06 AM

Archived from groups: rec.games.roguelike.development (More info?)

"copx" <invalid@invalid.com> wrote in
news:D h7c29$53j$04$1@news.t-online.com:
> Let's look at the abomination one last time:
>
> r_ptr = &r_info[m_ptr->r_idx];
>
> and remember kids: don't try this at home or in your CVS
> repository.


Yes, it has four underscores. Pretty bad.

BTW not everyone uses CVS. Too easy to mix you old bad code
with your new bad code when you forgot all that sideefects of
your old code.
!