views:

252

answers:

2

Ch 7.6 of Code Complete 2 is confusing me, I've attached some sample code (in php) mind telling me which style is the best? or suggest something better? thanks

Style 1

public function register($user, $pass) {
 if($this->model->isRegistered($user)
 {
  return false;
 }
 else if($this->twitter->login($user, $pass))
 {
  return $this->model->addUser($user, $pass);
 }

 return false;
}

Style 2

public function register($user, $pass) {
 if($this->model->isRegistered($user)
 {
  return false;
 }

 $this->twitter->login($user, $pass);
 if($this->twitter->isLoggedIn())
 {
  return $this-model->addUser($user, $pass);
 }

 return false;
}

Style 3

public function register($user, $pass) {
 if($this->model->isRegistered($user)
 {
  return false;
 }

 $status = $this->twitter->login($user, $pass);
 if($status)
 {
  return $this->model->addUser($user, $pass);
 }

 return false;
}

I'm currently making use of Style 1. Though I'm not quite sure if its the right one.

+2  A: 

In Style 1 "if" and "else if" is used on different conditions, so it doesn't make sense.

In Style 2 lines:

 $this->twitter->login($user, $pass);
 if($this->twitter->isLoggedIn())

are too much hard to read in some situations but it's a proper.

For me the best one is Style 3.

Kamilos
By definition `else if` always has a different condition than the preceding `if`, so your objection doesn't make sense :-)
Zano
+3  A: 

I don't want to sound too rude but I like noone of the 3 proposed styles. If I'm checking for conditions preventing the execution of a function, I'll always stick with this style. In general:

function action()
{
    if ($guard_condition1)
        return $failure;

    if ($guard_condition2)
        return $failure;

    do_action();
    return $success;
}

So I'd rewrite your code as following:

public function register($user, $pass)
{
    if ($this->model->isRegistered($user))
        return false;

    if (!$this->twitter->login($user, $pass))
        return false;

    return $this->model->addUser($user, $pass);
}

Anyway, if you need an opinion on what you proposed, I'd vote for style 3.

ntd
no worries, i'm open to suggestions lol
lemon