coding competition

[neotokyo]

for(uint32 i=0; i!=max_door_type;i++)
{
const Door* door = GetDoorDBID(i);
if(door == 0 || door->db_id == 0 || strcasecmp(door->zone_name, zone_name))
continue;
if(door->door_id == door_id && strcasecmp(door->zone_name, zone_name) == 0)
return door;
}

whats wrong with these few lines of code ...?
winner gets a free ticket to new delhi, india.

(and btw, this is not to diss anybody, but perhaps people will learn to code if we get the most commonly errors communicated)
*edit: feel free to post my errors too, if you find some.*

[Lurker_005]

Ok, not a coder, but I like to play with it anyhow :p

Shouldn't it be checking for true?
if(door->door_id == door_id && strcasecmp(door->zone_name, zone_name) == 1)

and why is it running thru door types and not door ID's?
for(uint32 i=0; i!=max_door_type;i++)

[neotokyo]

close, but no cigar

string-compare functions always return 0 if strings are equal - so that is ok.
max_door_type is a bad name, but other than that its used correctly.

[Bardboy]

Maybe I can answer that next semester, when I take C Programming I class.

[Trumpcard]

This might not be the answer youre looking for, but using != max is a bit dangerous.
If max gets set to a value other than an integer, or the comparison fails to catch the exit condition, which might be easy to do, this is an infinite loop.

Safer practice would be <=max_type

[neotokyo]

trump: right on both accounts. it is much safer, and it isnt the answer i am looking for

[kathgar]

/*guessing*/Door * GetDoorByDoorID(const *zone_name,uint32 door_id)
{/*god I hate editing code this way*/
for(uint32 i=0; i!=max_door_type;i++)
{
const Door* door = GetDoorDBID(i);
/*if door is undefined you try to dereference this=bad+crash */
//if(door == 0 || door->db_id == 0 || strcasecmp(door->zone_name, zone_name))
if(door){if(door->db_id==door_id && strcasecmp(door->zone_name, zone_name)==0)
return door;//continue;
//if(door->door_id == door_id/*this doesnt' exist*/ && strcasecmp(door->zone_name, zone_name) == 0)
//return door;}
} 
return NULL;//if we don't find it
}

clean block

Door * GetDoorByDoorID(const *zone_name,uint32 door_id)
{/*god I hate editing code this way*/
         for(uint32 i=0; i!=max_door_type;i++)
         {
                  const Door* door = GetDoorDBID(i);
                  if(door)
                  {
                        if(door->db_id==door_id && strcasecmp(door->zone_name, zone_name)==0)
                               return door;
                  }
         } 
         return NULL;//if we don't find it
}

Neo: read the post on the dev board about looking for new devs, then you get CVS access <3

[neotokyo]

do i get a cool board title and avatar then? )

and the '@' in irc?

and a trailer and a cook and daily massage?

btw. kath: the code is still wrong but much better then before

[Trumpcard]

Yep.. And you can boot me out of IRC when I post too many crappy code changes!

[kathgar]

anyone can change their avatar and title... and you would get probably %(halfop.. topic kick ban, just not halfop or op others)

[kathgar]

/*guessing*/Door * GetDoorByDoorID(const *zone_name,uint32 door_id)
{/*god I hate editing code this way*/
for(uint32 i=0; i!=max_door_type;i++)
{
const Door* door = GetDoorDBID(i);
/*if door is undefined you try to dereference this=bad+crash */
//if(door == 0 || door->db_id == 0 || strcasecmp(door->zone_name, zone_name))
if(door){if(door->db_id==door_id && strcasecmp(door->zone_name, zone_name)==0)
return door;//continue;
//if(door->door_id == door_id/*this doesnt' exist*/ && strcasecmp(door->zone_name, zone_name) == 0)
//return door;}
} 
return NULL;//if we don't find it
}

clean block

Door * GetDoorByDoorID(const *zone_name,uint32 door_id)
{/*god I hate editing code this way*/
         Door* door = NULL;
         for(uint32 i=0; i<max_door_type;i++)
         {
                  door = GetDoorDBID(i);
                  if(door)
                  {
                        if(door->db_id==door_id && strcasecmp(door->zone_name, zone_name)==0)
                               return door;
                  }
         } 
         return NULL;//if we don't find it
}

Thats all I see at the moment.. now i'll get some breakfast.. er lunch.. wahtever

[alkrun]

for(uint32 i=0; i <= max_door_type; i++)
{
Door* door = GetDoorDBID(i);
if(door == NULL)
continue;

if(door->db_id == 0 || strcasecmp(door->zone_name, zone_name))
continue;

if(door->door_id == door_id && strcasecmp(door->zone_name, zone_name) == 0)
return door;
}



My personal preference... I hate using 0 in the place of NULL. it's one of my biggest complaints about the emu sourcecode. Since you all don't use hungarian, sometimes it's not directly obvious if a variable is an id or a pointer. If you see this:

if(door == 0)

door could be either. If you see this:

if(door = NULL)

then it's pretty obvious that it's a pointer.

Another of my pointer issues is testing for null in the same conditional as you use to test for a member's value like the original code was testing door and then the door's database id. I prefer to see them split up like I have them. Again, it's a style thing, but with so many people working on the same code, it's easy to get someone to add a condition to your statement and make a mistake.

[kathgar]

Not just a style thing, if you test if the pointer is null and dereference it in the same if it tries to dereference if it is null.. which is quite bad

[neotokyo]

Originally Posted by kathgar

Door * GetDoorByDoorID(const *zone_name,uint32 door_id)
{/*god I hate editing code this way*/
         Door* door = NULL;
         for(uint32 i=0; i<max_door_type;i++)
         {
                  door = GetDoorDBID(i);
                  if(door)
                  {
                        if(door->db_id==door_id && strcasecmp(door->zone_name, zone_name)==0)
                               return door;
                  }
         } 
         return NULL;//if we don't find it
}

we have a winner )

the problem was:

Door* door = GetDoorDBID(i);

since the for-loop doesnt leave the scope, the variable door is intialized using GetDoorDBID(i) (which in this case i == 0), and then never again touched. so all you do is that you run the loop thru from i=0 to i=max_door_type, but with always the same door.


take care,
neo

[Baron Sprite]

% is sexy
oh forgot:

#include <iostream.h>

int main()
{
    std::cout << "Hello, world!\n";
}