eliminating redundancy

My current setup allows user to register in the application with a username, a password and email address. After registration, a user can add multiple addresses to his/her account. I want to allow users to login by username or any of the many email addresses associated with their account.

My UserIdentity file is like this




	const ERROR_EMAIL_ADDRESS_INVALID=3;

	

	public $email_address;

	

	private $_id;

	

	public function __construct($login, $password)

	{

		if(strpos($login, '@'))

		{

			$this->email_address = $login;

		}else{

			$this->username = $login;

		}

		$this->password = $password;

	}


	public function authenticate()

	{

		if($this->email_address !== null)

		{

			$email = UserEmail::model()->with(array('user'))->findByAttributes(array('email_address'=>$this->email_address));

			if($email === null || !isset($email->user))

				$this->errorCode=self::ERROR_EMAIL_ADDRESS_INVALID;

			else if($email->user->password !== $email->user->encryptPassword($this->password))

			{

				$this->errorCode=self::ERROR_PASSWORD_INVALID;

			}else{

				$this->_id = $email->user->id;

				$this->errorCode=self::ERROR_NONE;

				$email->user->addLogin();

			}

		}else{

			$record=User::model()->findByAttributes(array('username'=>$this->username));

		

			if($record===null)

			{

				$this->errorCode=self::ERROR_USERNAME_INVALID;

			}else if($record->password!==$record->encryptPassword($this->password))

			{

				$this->errorCode=self::ERROR_PASSWORD_INVALID;

			}else{

				$this->_id = $record->id;

				$this->errorCode=self::ERROR_NONE;

				$record->addLogin();

			}

		}

		return !$this->errorCode;

	}



As you can see, there is redundant code. Can anyone think of a way to avoid this redundancy, so far i have found no better way than this.

Just for clarity, this is what the tables look like




CREATE TABLE `user` (

  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,

  `primary_email_id` int(10) unsigned DEFAULT NULL,

  `username` varchar(32) NOT NULL,

  `password` varchar(32) NOT NULL,

  `status` tinyint(1) unsigned NOT NULL,

  `create_time` int(10) unsigned NOT NULL,

  `create_ip` varchar(39) NOT NULL,

  `last_update_time` int(10) unsigned DEFAULT NULL,

  `last_active_time` int(10) unsigned DEFAULT NULL,

  PRIMARY KEY (`id`),

  UNIQUE KEY `username` (`username`)

) ENGINE=InnoDB  DEFAULT CHARSET=utf8;





CREATE TABLE `user_email` (

  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,

  `user_id` int(10) unsigned NOT NULL,

  `email_address` varchar(50) NOT NULL,

  `confirmed` tinyint(1) NOT NULL,

  `create_time` int(10) unsigned NOT NULL,

  PRIMARY KEY (`id`),

  UNIQUE KEY `email_address` (`email_address`)

) ENGINE=InnoDB  DEFAULT CHARSET=utf8;




ALTER TABLE `user_email`

  ADD CONSTRAINT `FK_user_email_user` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE ON UPDATE CASCADE;




If you don’t like the redundant code… you can do something like that…




public function authenticate()

{

    $this->errorCode=self::ERROR_NONE;


    if($this->email_address !== null)

    {

        $email=UserEmail::model()->with(array('user'))->findByAttributes(array('email_address'=>$this->email_address));

        if($email === null || !isset($email->user))

            $this->errorCode=self::ERROR_EMAIL_ADDRESS_INVALID;

        else 

            $record=$email->user;

    }

    else

    {

        $record=User::model()->findByAttributes(array('username'=>$this->username));

        if($record === null)

        {

            $this->errorCode=self::ERROR_USERNAME_INVALID;

        }

    }


    if($this->errorCode==self::ERROR_NONE;)

    {

        if($record->password !== $record->encryptPassword($this->password))

        {

            $this->errorCode=self::ERROR_PASSWORD_INVALID;

        }

        else

        {

            $this->_id=$record->id;

            $record->addLogin();

        }

    }

    return!$this->errorCode;

}



My suggestions:

  1. Move primary_email_id to user_email and make it an indicator. It doesn’t belong in the user table.

  2. Alter your sql as follows:

Select * from user where id in ( select user_id from user_email where email_address = :searchValue ) or username = :searchValue

Obviously assume username and email_address are unique columns. You can use User::model()->findBySql or plain PDO.

  1. Validate results

I have thought about doing that, but that would mean that more than one email address can be set as primary at any one time. I think having it in user table is a viable option in this scenario.

Thanks mdomba, that solved my problem.