Question return; question

JTP
Topic Author
Offline
Platinum Member

29 Apr 2016 19:11 - 29 Apr 2016 19:16 #5801 by JTP

return; question was created by JTP

Below is a part of 3 skills, first one uses no return; The other only uses one near the end. And the last uses alot of return;

So when is it a good idea to use and when is it not ?

Code was found on circlemud, but made me wonder why the differences.

Code:

  if (roll <= goal) {
    success = TRUE;
    if ((weap = GET_EQ(vict, WEAR_DWIELD))) {
      if (IS_NPC(vict))
        LOST_WEAPON(vict) = weap;

      act("You disarm $p from $N's off-hand!", FALSE, ch, weap, vict, TO_CHAR);
      act("$n disarms $p from your off-hand!", FALSE, ch, weap, vict, TO_VICT);
      act("$n disarms $p from $N's off-hand!", FALSE, ch, weap, vict, TO_NOTVICT);
      obj_to_room(unequip_char(vict, WEAR_DWIELD), IN_ROOM(vict));
    } else if ((weap = GET_EQ(vict, WEAR_WIELD))) {
      if (IS_NPC(vict))
        LOST_WEAPON(vict) = weap;

      act("You disarm $p from $N's hand!", FALSE, ch, weap, vict, TO_CHAR);
      act("$n disarms $p from your hand!", FALSE, ch, weap, vict, TO_VICT); 
      act("$n disarms $p from $N's hand!", FALSE, ch, weap, vict, TO_NOTVICT);
      obj_to_room(unequip_char(vict, WEAR_WIELD), IN_ROOM(vict));
    } else {
      log("SYSERR: do_disarm(), should have a weapon to be disarmed, but lost it!");
    }
  } else {
      act("You fail to disarm $N.", FALSE, ch, weap, vict, TO_CHAR);
      act("$n fails to disarm you.", FALSE, ch, weap, vict, TO_VICT);
      act("$n fails to disarm $N.", FALSE, ch, weap, vict, TO_NOTVICT);
  }

  if (!IS_IMMORT(ch))
    WAIT_STATE(ch, PULSE_VIOLENCE);

  if (success && IS_NPC(vict))
    set_fighting(ch, vict);
}

Code:

  if (!obj)
  GET_MOVE(ch) -= (GET_LEVEL(ch) / 2) + (GET_DEX(ch) * 2);
  if (!obj)
  GET_HIT(ch) += (GET_LEVEL(ch) / 2) + (GET_DEX(ch) * 2);
  else
  GET_HIT(ch) += (GET_LEVEL(ch) / 2);
  if (!obj) {
  send_to_char("You bandage yourself.\r\n", ch);
  act("$n bandages $mself.", FALSE, ch, 0, 0, TO_ROOM);
} else {
  act("You bandage yorself with $p.", FALSE, ch, obj, 0, TO_CHAR);
  act("$n bandages $mself with $p.", FALSE, ch, obj, 0, TO_ROOM);
  extract_obj(obj); }

  if (GET_HIT(ch) >= GET_MAX_HIT(ch)) {
  send_to_char("You are now at full health.\r\n", ch);
  GET_HIT(ch) = GET_MAX_HIT(ch); }

  WAIT_STATE(ch, PULSE_VIOLENCE * 1);
  return;
 }
}

Code:

   if(!*buf) {
      /* no argument means that the character is listening for
       * hidden or invisible beings in the room he/she is in
       */
      for(tch = world[ch->in_room].people; tch; tch = tch_next) {
         tch_next = tch->next_in_room;
         if((tch != ch) && !CAN_SEE(ch, tch) && (GET_LEVEL(tch) < LVL_IMMORT))
            found++;
      }
      if(found) {
         if(GET_LEVEL(ch) >= 15) {
            /* being a higher level is better */
            sprintf(buf, "You hear what might be %d creatures invisible, or hiding.\r\n", \
                        MAX(1,(found+number(0,1)-number(0,1))));
         }
         else sprintf(buf, "You hear an odd rustling in the immediate area.\r\n");
         send_to_char(buf, ch);
      }
      else send_to_char(heard_nothing, ch);
      act(room_spiel, TRUE, ch, 0, 0, TO_ROOM);
      return;
   }
   else {
      /* the argument must be one of the cardinal directions: north,
       * south, etc.
       */
      for(dir = 0; dir < NUM_OF_DIRS; dir++) {
         if(!strncmp(buf, dirs[dir], strlen(buf)))
            break;
      }
      if (dir == NUM_OF_DIRS) {
         send_to_char("Listen where?\r\n", ch);
         return;
      }
      if(CAN_GO(ch, dir) || CAN_LISTEN_BEHIND_DOOR(ch, dir)) {
         for(tch = world[EXIT(ch, dir)->to_room].people; tch; tch=tch_next) {
            tch_next = tch->next_in_room;
            found++;
         }
         if(found) {
            if(GET_LEVEL(ch) >= 15) {
               sprintf(buf, "You hear what might be %d creatures %s%s.\r\n", \
                        MAX(1,(found+number(0,1)-number(0,1))),
                        ((dir==5)?"below":(dir==4)?"above": "to the "),
                        ((dir==5)?"":(dir==4)?"":dirs[dir]));
            }
            else sprintf(buf, "You hear sounds from %s%s.\r\n", \
                        ((dir==5)?"below":(dir==4)?"above": "the "),
                        ((dir==5)?"":(dir==4)?"":dirs[dir]));
            send_to_char(buf, ch);
         }
         else send_to_char(heard_nothing, ch);
         act(room_spiel, TRUE, ch, 0, 0, TO_ROOM);
         return;
      }
      else send_to_char("You can't listen in that direction.\r\n", ch);
      return;
   }
   return;
}

Last edit: 29 Apr 2016 19:16 by JTP.

Please Log in or Create an account to join the conversation.

thomas
Offline
Administrator

29 Apr 2016 21:39 #5803 by thomas

Replied by thomas on topic return; question

This is a hornets' nest, I'm afraid. There are two schools in programming - those who use at most one return statement in a function, and those who use more.

In general, this particular sequence of characters at the end of a function:

Code:

    return;
  }

(note that the } is indented two characters less than the return statement) is a sign that the programmer was inexperienced at the time of writing. There are exceptions (notably, if the } marks the end of a loop body), but generally, the return is a no-op - it makes no difference.

Some simplified examples:

Code:

// foo has no returns. It will implicitly return at the end of the function.
void foo1() {
  if (x) {
    bar();
  } else {
    baz();
  }
}

Code:

// This is another version of foo() - it does the same as above.
// note that now we have an explicit return in addition to the implicit at the end.
void foo2() {
  if (x) {
    bar();
    return;
  } 
  baz();
}

Code:

// yet another version of foo() - it does the same as above.
// Now, both the returns are explicit. 
// The compiler will remove the last return from the compiled code, so it makes no difference

void foo3() {
  if (x) {
    bar();
    return;
  } 
  baz();
  return;
}

Code:

// Here's another function. It uses a return in the loop to stop the function when the condition is met.
// The implicit return at the end is not used.
void baz1() {
  while (true) {
    bar();
    if (!x) {
      return;
    }
  } 
}

Code:

// Here's another function. It uses a break in the loop to stop the function when the condition is met.
// This triggers the implicit return at the end.
void baz2() {
  while (true) {
    bar();
    if (!x) {
      break;
    }
  } 
}

So, which is better? Well, I think it depends.

The foo1() style is pretty, easily readable if there's only one or two conditions and even old compilers can make it into fast code. However, if you have several conditions, it ends up something like this:

Code:

// foo has no returns. It will implicitly return at the end of the function.
void foo11() {
  if (x) {
    if (y) {
      bar();
    } else {
      if (z) {
        top();
      } else {
        des();
      }
    }
  } else {
    baz();
  }
}

This can be both hard to read (especially if you mess up the indentation) and easy to mess up. In short, this is a candidate for brain overload. The foo2() method fixes those things:

Code:

void foo12() {
  if (!x) {
    baz();
    return;
  }
  if (y) { 
    bar();
    return;
  }
  if (z) { 
    top();
    return;
  } 
  des(); 
}

The "return early" avoids deep nesting of potentially long if/else if/else statements. As a tradeoff, it makes for longer code. Modern compilers will compile it to the same machine code as the code above.

About foo3() - this is typical of inexperienced programmers, or programmers new to C (or Java, which has the same semantics).

About baz1() vs baz2() - generally, use break, unless you absolutely must avoid running some code after the loop:

Code:

// Here's another function. It uses a break in the loop to stop the function when the condition is met.
// This makes it run the tpi(); call.
void baz3() {
  while (true) {
    bar();
    if (!x) {
      break;
    }
  } 
  tpi(); // this code will run because of the break - not if using return
}

Please Log in or Create an account to join the conversation.

JTP
Topic Author
Offline
Platinum Member

29 Apr 2016 22:03 - 29 Apr 2016 22:44 #5804 by JTP

Replied by JTP on topic return; question

Question 1:

Can easily see a good point in using them early on.

Code:

  if (IS_NPC(ch) || !GET_SKILL(ch, SKILL_****)) {
	send_to_char(ch, "You have no idea how to do that.\r\n");
	return;
  }
  if (GET_MOVE(ch) <50) {
	send_to_char(ch, "You're too exhausted!\r\n");
	return;
  }

Can one use to many return;'s ? Is this even nessesary ?

Code:

     send_to_char(ch, "You fail...\r\n");
     act("$n fails.\r\n", FALSE, ch, 0, 0, TO_ROOM);
     return;
    } else {
     send_to_char(ch, "You succedd!\r\n");
     act("$n succeeds!\r\n", FALSE, ch, 0, 0, TO_ROOM); 
     return;

  }
  return; <<--and here in the end ?
}

Last edit: 29 Apr 2016 22:44 by JTP.

Please Log in or Create an account to join the conversation.

JTP
Topic Author
Offline
Platinum Member

29 Apr 2016 22:24 - 29 Apr 2016 22:45 #5805 by JTP

Replied by JTP on topic return; question

Question 2:

Now that were at it, im using ver 3.65 since iwe been tinckling with coding on and off, my daughter is very into D&D, so all for her haha, and today i ran a compile with -Wextra

And got like 100 messages like :

act.comm.c: In function ‘do_say’:
act.comm.c:40: warning: unused parameter ‘cmd’
act.comm.c:40: warning: unused parameter ‘subcmd’
act.comm.c: In function ‘do_gsay’:
act.comm.c:74: warning: unused parameter ‘cmd’
act.comm.c:74: warning: unused parameter ‘subcmd’
act.comm.c: In function ‘do_tell’:
act.comm.c:144: warning: unused parameter ‘cmd’
act.comm.c:144: warning: unused parameter ‘subcmd’
act.comm.c: In function ‘do_reply’:
act.comm.c:198: warning: unused parameter ‘cmd’
act.comm.c:198: warning: unused parameter ‘subcmd’

and

act.informative.c: In function ‘do_gold’:
act.informative.c:800: warning: unused parameter ‘argument’
act.informative.c:800: warning: unused parameter ‘cmd’
act.informative.c:800: warning: unused parameter ‘subcmd’
act.informative.c: In function ‘do_score’:
act.informative.c:961: warning: unused parameter ‘argument’
act.informative.c:961: warning: unused parameter ‘cmd’
act.informative.c:961: warning: unused parameter ‘subcmd’
act.informative.c: In function ‘do_inventory’:
act.informative.c:1167: warning: unused parameter ‘argument’
act.informative.c:1167: warning: unused parameter ‘cmd’
act.informative.c:1167: warning: unused parameter ‘subcmd’

And ALOT of othere, this is just a few.
Whats up with that ?

And some others:

spec_procs.c: In function ‘list_skills’:
spec_procs.c:114: warning: comparison of unsigned expression < 0 is always false

dg_olc.c: In function ‘format_script’:
dg_olc.c:925: warning: comparison of unsigned expression < 0 is always false

dg_scripts.c: In function ‘dg_letter_value’:
dg_scripts.c:2439: warning: comparison between signed and unsigned

genobj.c: In function ‘oset_alias’:
genobj.c:490: warning: comparison between signed and unsigned
genobj.c: In function ‘oset_short_description’:
genobj.c:561: warning: comparison between signed and unsigned
genobj.c: In function ‘oset_long_description’:
genobj.c:579: warning: comparison between signed and unsigned

house.c: In function ‘House_save_control’:
house.c:222: warning: comparison between signed and unsigned

oasis_list.c: In function ‘list_rooms’:
oasis_list.c:602: warning: comparison between signed and unsigned
oasis_list.c: In function ‘list_mobiles’:
oasis_list.c:647: warning: comparison between signed and unsigned
oasis_list.c: In function ‘list_objects’:
oasis_list.c:694: warning: comparison between signed and unsigned

protocol.c: In function ‘CompressStart’:
protocol.c:59: warning: unused parameter ‘apDescriptor’
protocol.c: In function ‘CompressEnd’:
protocol.c:70: warning: unused parameter ‘apDescriptor’
protocol.c: At top level:
protocol.c:194: warning: missing initializer
protocol.c:194: warning: (near initialization for ‘VariableNameTable[61].bConfigurable’)
protocol.c: In function ‘SoundSend’:
protocol.c:1389: warning: comparison between signed and unsigned
protocol.c: In function ‘PerformSubnegotiation’:
protocol.c:1702: warning: unused parameter ‘aSize’
protocol.c: In function ‘SendMSSP’:
protocol.c:2256: warning: missing initializer
protocol.c:2256: warning: (near initialization for ‘MSSPTable[0].pFunction’)
protocol.c:2261: warning: missing initializer
protocol.c:2261: warning: (near initialization for ‘MSSPTable[3].pFunction’)
protocol.c:2359: warning: missing initializer
protocol.c:2359: warning: (near initialization for ‘MSSPTable[4].pFunction’)

utils.c: In function ‘column_list’:
utils.c:1032: warning: comparison between signed and unsigned
utils.c:1045: warning: comparison between signed and unsigned
utils.c:1079: warning: comparison between signed and unsigned

Last edit: 29 Apr 2016 22:45 by JTP.

Please Log in or Create an account to join the conversation.

thomas
Offline
Administrator

30 Apr 2016 14:00 #5807 by thomas

Replied by thomas on topic return; question

All of the "commands" are created using the macro ACMD(command), which the preprocessor (see here: github.com/tbamud/tbamud/blob/master/src/utils.h#L27 ) turns into

Code:

void command(struct char_data *ch, char *argument, int cmd, int subcmd)

If the actual function doesn't use all of those parameters, you will get the "unused parameter" warning when you compile with -Wunused (which is included in -Wall).
This is a design decision - to make it easier to identify a command and to make sure all commands have the same interface.

About the "comparison between signed and unsigned" - see this SO question: stackoverflow.com/questions/25345883/com...s-wsign-compare-warn

About "missing initializer": Never seen it before. Seems not to be important.
About "comparison of unsigned expression < 0 is always false" : github.com/tbamud/tbamud/blob/master/src/dg_olc.c#L1155 checks for ret < 0, but ret is unsigned so that is always false. The check is redundant, but not dangerous.

Please Log in or Create an account to join the conversation.

thomas
Offline
Administrator

30 Apr 2016 14:00 #5808 by thomas

Replied by thomas on topic return; question

All of the "commands" are created using the macro ACMD(command), which the preprocessor (see here: github.com/tbamud/tbamud/blob/master/src/utils.h#L27 ) turns into

Code:

void command(struct char_data *ch, char *argument, int cmd, int subcmd)

Please Log in or Create an account to join the conversation.

Time to create page: 0.203 seconds