Device limitation rule for user I've created a simple application for mobile users, they can login using provided API but must have limitation on the devices, so the requirement is simple : 


  User cannot login using more than 2 devices


I'm building the API using laravel for mobile apps, so I've created the function and it works correctly and wrote test and it works fine you can find the full code here.

But I would like to know what are you guys think of this code : 

public function isLoginValid2($user_id,$device_id){

        $deviceLimit = 2;

        //retrive device or create new one if doesnt exist
        $params = [
            'user_id' =&gt; $user_id,
            'device_id' =&gt; $device_id
        ];

        $device = Device::where($params)-&gt;first();

        if (!$device) {

            // check if not exceed limit  
            if (Device::where(['user_id'=&gt;$user_id, 'is_banned'=&gt;false])-&gt;count() &lt;= $deviceLimit) {
                return true;
            }else{
                // the device is above llmit
                return false;
            }

        }else {
            // check if banned
            if ($device-&gt;is_banned) {
                // the device is banned user is not pemmitted to login
                return false;
            }else{
                return true;
            }
        }

    }


As for the code above if we take a look there are some code line that we can modify or move to create another function.

The reason why I did it because I can make a unit test case on every function and make sure everything is work. so the code above I've modified it and become like this : 

public function isLoginValid2Refactored($user_id,$device_id){
        if($this-&gt;isDeviceExistWithUser($user_id,$device_id)){
            //device is exist check if its banned
            if($this-&gt;isDeviceBanned($device_id)){
                return false;
            }else{
                return true;
            }
        }else{
            //device is not exits check device limit
            if($this-&gt;isUserReachLimitDevice($user_id)){
                //user reach limit device 
                return false;
            }else{
                return true;
            }
        }
    }


So from here I created another function called isDeviceExistWithUser and isUserReachLimitDevice any better idea to write testable?  Or any better improvement so I others can read it easily?
$device = Device::where($params)-&gt;first(); will either return an instance of eloquent or null.

You have if (!$device) {...}else{...}. 

This translates to if the device is not found aka the else part then $device-&gt;is_banned which will results in a trying to get property of non-object fatal error.



if ($condition) {
    return false;
}else{
    return true;
}


The else is not necessary since return will immediately end the execution of the function and return the result.

if ($condition) {
    return false;
}
return true;


Works just the same and I personally find it more visually pleasant.



$this-&gt;isDeviceExistWithUser($user_id,$device_id)

Syntax heavy naming. You could simple call it deviceForUser or something like that. is usually suggests a boolean, as in isDevice, isSmall, isLarge etc etc.



$this-&gt;isDeviceBanned($device_id)

Nitpick but a simple isBanned will do here since you passing in $device_id, clearly a device is the scope here.



isLoginValid2Refactored

Again really heavy name here. It's not something inherently wrong with it, but the lighter the better. Fluent and easy to understand goes a long way.