Refactoring method?

Question

In TokenRepository you can see 3 similar methods. It creates a new entry in the token table, but each method has different fields.

How can I reorganize this? Should I combine 3 methods into one method or use a strategy template?

TokenRepository Class:

 class TokenRepository { public function createTokenDigitalOcean(User $user, $name, $accessToken, $refreshToken = null) { return $user->tokens()->create([ 'name' => $name, 'provider' => 'digital_ocean', 'access_token' => $accessToken, 'refresh_token' => $refreshToken, ]); } public function createTokenLinode(User $user, $name, $key) { return $user->tokens()->create([ 'name' => $name, 'provider' => 'linode', 'linode_key' => $key, ]); } public function createTokenAws(User $user, $name, $key, $secret) { return $user->tokens()->create([ 'name' => $name, 'provider' => 'aws', 'aws_key' => $key, 'aws_secret' => $secret, ]); } } 

I have 3 classes like DigitalOceanProvider , LinodeProvider and AwsProvider . For example, to use the LinodeProvider and AwsProvider classes.

 class LinodeProvider { public function callback() { $this->tokenRepo->createTokenLinode($user, $name, $key); } } class AwsProvider { public function callback() { $this->tokenRepo->createTokenAws($user, $name, $key, $secret); } } 

Answer 1

It may be a little outwit, but in order to make life a little easier in the future, you can create separate implementations of each of them that extend the abstract class. This way you can combine and define an interface and easily add new types of tokens.

 <?php namespace Foo\Tokens; abstract class Token { protected $name = ''; protected $key = ''; protected $provider = ''; public function __construct($name, $key) { $this->name = $name; $this->key = $key; } public function data() { return [ 'name' => $this->name, 'provider' => $this->provider, 'token' => $this->key ]; } } 

Then we create our Digital Ocean token class. This class can either use the default implementation or override it.

 <?php namespace Foo\Tokens; use Foo\Tokens\Token; class DigitalOceanToken extends Token { protected $provider = 'digital_ocean'; public function __construct($name, $key, $refreshToken = null) { parent::__construct($name, $key); $this->refreshToken = $refreshToken; } public function data() { return [ 'name' => $this->name, 'provider' => $this->provider, 'key' => $this->key, 'refreshToken' => $this->refreshToken ]; } } 

Now TokenRepository just takes care of attaching this token to the user.

 <?php namespace Foo; use User; use Foo\Tokens\Token; class TokenRepository { public function createToken(User $user, Token $token) { return $user->tokens()->create( $token->data() ); } } 

And your service providers are as simple as ...

 <?php use Foo\Tokens\AwsToken; class AwsProvider { public function callback() { $this->tokenRepo->createToken( $user, new AwsToken($name, $key, $secret) ); } } 

This is not working code, as I have not tried to run it, but it is just another idea on how you can organize and assign responsibility. Hope this helps, and welcome feedback from others.

Answer 2

For me, you should implement it like this:

 class TokenRepository { public function createTokenForVendor(User $user, $inputs) { return $user->tokens()->create($inputs); } } 

and inside your callback:

 class VendorProvider { public function callback() { switch($tokenType) { case 'DigitalOcean': $inputs = [ 'name' => $name, 'provider' => 'digital_ocean', 'access_token' => $accessToken, 'refresh_token' => $refreshToken, ]; break; case 'Linode': $inputs = [ 'name' => $name, 'provider' => 'linode', 'linode_key' => $key, ]; break; case 'Aws': $inputs = [ 'name' => $name, 'provider' => 'aws', 'aws_key' => $key, 'aws_secret' => $secret, ]; break; } $this->tokenRepo->createTokenForVendor($user, $inputs); } } 

We hope you need to do some modernization of the code structure.

Hope this helps!