Within a PHP class is it better to return values from a private method or is it better to update private property with results and refer to them?

I am thinking in terms of readability and performance. So is it better to write the code like so:

Option A

class Registrar 
{
    /**
     * @var null|\App\Contracts\Passkey\PasskeyUser $passkeyUser
     */
    private PasskeyUser $passkeyUser;

    /**
     * @var string $challengeBytes
     */
    private $challengeBytes;

    /**
     * @var PublicKeyCredentialCreationOptions $publicKeyCredentialCreationOptions
     */
    private PublicKeyCredentialCreationOptions $publicKeyCredentialCreationOptions;

    /**
     * @var mixed $serializedOptions
     */
    private $serializedOptions;

    /**
     * Generate registration options
     * 
     * @param null|\App\Contracts\Passkey\PasskeyUser $passkeyUser
     * @return mixed
     */
    public function generateOptions(?PasskeyUser $passkeyUser)
    {
        $this->setUser($passkeyUser);

        $this->generateChallenge();

        $this->generatePublicKeyCredentialCreationOptions();

        return $this->serializeOptions();
    }

    /**
     * Generate a challenge
     */
    private function generateChallenge()
    {
        $this->challengeBytes = random_bytes(16);
    }

    // more private functions below
}

Or is it better to write like so:

Option B

class Registrar 
{
    /**
     * @var null|\App\Contracts\Passkey\PasskeyUser $passkeyUser
     */
    private PasskeyUser $passkeyUser;

    /**
     * Generate registration options
     * 
     * @param null|\App\Contracts\Passkey\PasskeyUser $passkeyUser
     * @return mixed
     */
    public function generateOptions(?PasskeyUser $passkeyUser)
    {
        $this->setUser($passkeyUser);

        $challengeBytes = $this->generateChallenge();

        $pkCreationOptions = $this->generatePublicKeyCredentialCreationOptions($challengeBytes);

        return $this->serializeOptions($pkCreationOptions);
    }

    /**
     * Generate a challenge
     */
    private function generateChallenge()
    {
        return random_bytes(16);
    }

    // more private functions below
}

This is not an issue. Just a question on what is the best way to do it. I am trying to figure out what is better in terms of:

Readability
Performance
Memory

Thoughts so far

I believe Option A is better readable while Option B avoids the requirement to pass variables back and forth where it is not really needed coz everything happens in the same class.

Tray2

2 years ago

Level 73

for readability I don't see that much difference, and for performance, I doubt the parser even cares which way you do the code. Memory, yeah maybe if you use a lot of temororary variables, but it seems that we are looking at bytes here...

Snapey

2 years ago

Best Answer

Level 122

creating class properties for parameter passing would seem no better than using local temp variables, but its actually a lot worse.

Using class properties means its harder to see whats going on.

I look at your class and think private $challengeBytes, ok, whats that for? ... goes hunting through the code for all the places it might be used.

Then when looking at a function, I see use of a class property in the function and I cant tell if I can trust that value. Could it be changed from elsewhere? I then go hunting in the code for other uses.

Is a variable needed at all?

return $this->serializeOptions(
        $this->generatePublicKeyCredentialCreationOptions(
            $this->generateChallenge()
        ) 
);

mathewparet

2 years ago

Level 22

Thanks guys.

Please or to participate in this conversation.