1
\$\begingroup\$

I need to log in users to web app in Javascript and PHP. So I have Javascript interface which send me accessToken to PHP which stores users data to DB and log user in. But the result code does not look like OOP but like a peace of garbage. May be I dont know how to write that code, so the question is if this peace of PHP code is ok or if it is possible to rewrite it to real OOP. Cause OOP exists because of people who use the code. It should make sense for humans. It seems this make sense only for authors of the code.

$code = $_POST['code'];


try
{
    $client = new Google_Client();
    $client->setAuthConfig(__DIR__ . '/../model/JsonData/GoogleCredentials.json');
    $client->setRedirectUri( self::REDIRECT_URI );

    $accessToken = $client->fetchAccessTokenWithAuthCode( $code );

    $client->setAccessToken( $accessToken );

    $peopleService = new \Google_Service_PeopleService( $client );
    $me = $peopleService->people->get('people/me', ['personFields' => 'emailAddresses,names,metadata,coverPhotos']);  // This is extra feature which does not accept spaces....

}
catch ( \Exception $e )
{
    Debugger::log( $e );
    $this->sendJson( array( 'errCode' => 1, 'error' => 'Log in fails.' ) );
}

/** @var \Google_Service_PeopleService_EmailAddress $email */
foreach ( $me->getEmailAddresses() as $email )
{
    if ( $email->getMetadata()->getPrimary() ) $user_email = $email->value;  // Oh really? I want to log in user....
}
/** @var \Google_Service_PeopleService_Name $name */
foreach ( $me->getNames() as $name )
{
    if ( $email->getMetadata()->getPrimary() ) $user_name = $name->displayName;
}

if ( ! isset( $user_email ) ) $this->sendJson( array( 'errCode' => 0, 'error' => 'Email is required.' ) );
if ( ! isset( $user_name ) ) $this->sendJson( array( 'errCode' => 0, 'error' => 'Name is required' ) );

// Hello ID is missing. How to uniquely identify user?

May be I am wrong but is it possible to write it better way to get relevant data for log in? Where is ID which can uniquely identify user?

Look at this JS interface. This looks like OOP https://developers.google.com/identity/sign-in/web/people

\$\endgroup\$
4
\$\begingroup\$

If I had to make this OOP, I'd begin by hedging concerns :

  • a class for user data handling (retrieving the POST and using it)
  • a service (that you'd dependency inject) to handle the configuration of the Google Service
  • a GoogleUser class for encapsulating the result of the service and abstracting all those foreach

Then, orchestrate those in some kind of controller (I guess you have already since you use a $this->sendJson method)

I like custom Exception cascade but that's just me :

try{
  //everything working well
}catch(GoogleLoginException $e){
}catch(RequiredEmailException $e){
}catch(RequiredNameException $e){
}

GoogleSDK is notoriously verbose and looks a lot like enterprise java and is always a pain to make elegant.

In the end you'd have something like :


class LoginController{
   private $google;

   public function __construct(GoogleService $google){
     $this->google = $google;
   }

   public function oauthCallbackAction($postRequest){
     $code = $postRequest->get("code");
     try{
         $authentifiedClient = $google->authentifiedClientWithCode($code);
         $googleUser = $authentifiedClient->getMe();
         return $this->json([
                              "user_name" => $googleUser->getUserName(),
                              "email" => $googleUser->getEmail(),
                            ], 200);
     }catch(GoogleAuthException $e){   //thrown by GoogleService
             return $this->json("blah", 500);
     }catch(RequiredEmailException $e){ //thrown by GoogleUser::getEmail
             return $this->json("gimme email !", 400);
     }catch(RequiredNameException $e){  //thrown by GoogleUser::getUserName
             return $this->json("gimme name !", 400);
     }

   }

}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ No this is not the point. Cause if it will be necessary to encapsulate all packages and its classes to another classes I end up with huge number of classes. It is not the goal of OOP. \$\endgroup\$ – Čamo Jul 8 at 9:33
  • 1
    \$\begingroup\$ Having a huge number of class is precisely the goal of OOP : each class should do a single thing and that thing well, and in the end, the orchestrator should be human readable because classes reflects the "business" \$\endgroup\$ – smwhr Jul 8 at 9:39
  • \$\begingroup\$ I dont think so. Every package should have its internal classes separated inside and give user public interface which will be readable by humans. This is not the case.... This is what we call haystack. \$\endgroup\$ – Čamo Jul 8 at 14:48
  • 1
    \$\begingroup\$ @Čamo public interface should be first of all easy to consume. The Google api is not easy to consume, that's why it Is a good idea to encapsilate it in a specialized interface that will be easy to consume by your specialized controller/service. You're asking for OOP approach, then dismiss the approach as unsuitable. What is it exactly that you want? \$\endgroup\$ – slepic Jul 9 at 4:24
  • \$\begingroup\$ What I want is to force people to use their brains if they produces code for others. Look at that Javascript example. Thats what I want. \$\endgroup\$ – Čamo Jul 9 at 7:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.