2

For example, for oauth, I may need to copy and paste oauth and oauth callback code like it (assume Client is the framework I use, I don't use real framework like google because I want to simplify the code):

sample code:

public function oauth(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(SOME_URL);
  //some $client settings
  header('Location: ' . filter_var($client->getURL(), FILTER_SANITIZE_URL));
}

public function oauth_callback(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(SOME_URL);
  //some $client settings
  if(!$_GET['code']){
        //back to oauth and return;
  }
  $userInfo=$client->getUserInfo();
}

In real code, I would copy and paste sample code like it:

Login:

public function login_oauth(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(MY_URL.'login_callback');
  //some $client settings
  header('Location: ' . filter_var($client->getURL(), FILTER_SANITIZE_URL));
}

public function login_callback(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(MY_URL.'login_callback');
  //some $client settings
  if(!$_GET['code']){
        //back to oauth and exit;
  }
  $userInfo=$client->getUserInfo($_GET['code']);
  //check login valid
}

Register:

public function register_oauth(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(MY_URL.'register_callback');
  //some $client settings
  header('Location: ' . filter_var($client->getURL(), FILTER_SANITIZE_URL));
}

public function register_callback(){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(MY_URL.'register_callback');
  //some $client settings
  if(!$_GET['code']){
        //back to oauth and exit;
  }
  $userInfo=$client->getUserInfo($_GET['code']);
  //check is user already registered
}

I know it is seriously violating DRY principle because I copy and paste code, so I try to extract common parts:

common functions:

public static function createClient($url){
  $client=new Client([MY_APP_ID,MY_APP_SECRET]);
  $client->setCallbackUrl(URL_PREFIX.$url);
  //some $client settings
  return $client;
}

public static oauth($url){
  $client=MyClass::createClient($url);
  header('Location: ' . filter_var($client->getURL(), FILTER_SANITIZE_URL));
}

public static oauth_callback($url){
  $client=MyClass::createClient($url);
  if(!$_GET['code']){
        //back to oauth and exit;
  }
  $userInfo=$client->getUserInfo($_GET['code']);
  return $userInfo;
}

Login:

public function login_oauth(){
  oauth('login_callback');
}

public function login_callback(){
  $userInfo=oauth_callback('login_callback');
  //check login valid
}

Register:

public function login_oauth(){
  oauth('login_callback');
}

public function login_callback(){
  $userInfo=oauth_callback('login_callback');
  //check is user already registered
}

But I found the refactored code is harder to maintain because:

  1. It looks quite different from the sample code, which is less understandable, especially for beginners who never uses this framework before

  2. The newer one has more sub functions, which spends far more time to link the relationship between those sub functions.

  3. When adding new API, using sample code is easy because I just need to copy and paste directly, but if I use my refactored version, I need to study how I refactored the code first.

  4. Refactoring sample code may introduce some new probems, eg: missing some line of codes during copying

  5. The code may be totally replaced by a newer version of sample code, or by other framework

So my question is, is 'Keep it looks similar to sample code' a valid reason to prefer WET over DRY principle?

1
  • 1
    Why would new engineer read sample code instead of yours?
    – Basilevs
    Commented Jul 11, 2018 at 3:24

1 Answer 1

9

No.

Indeed, following the DRY principle can sometimes make very simple code look a little bit more complex (which might be here the case, with emphasis on "a little bit").

However, this is a small price to pay when you need the same logic in more than one place of your system. Not centralizing it will have the impact of giving you several places to change in case you need to change that logic. And it bears a high risk of forgetting one of those places and introduce a bug that way.

Your example looks very clearly like one which falls under the latter category. Even in that small code snippet, it seems to be important that oauth_callback and oauth always use the same logic to create a $client object, so the logic should be in one place, a method createClient is the natural choice.

What you wrote gives me the impression your team is working on one software product, with the requirement of using the same authentication logic throughout the system in several places. So imagine what happens when the "beginners" you mentioned start to duplicated the createClient logic around to a dozen of places in the code base, and then you need to extend the initialization logic for the $client object. Do you really want to search for all those places and test them all, one by one? Sure you don't forget one in a huge code base, especially when the authentification code was modified somehow each time?

So better give those beginners (and everone else as well) some tested functions at hand which they can reuse without copying, instead of letting them produce a mess by copy-pasting the same logic over and over again.

See also:

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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