Refactoring PHP file to remove code duplication

Question

I have a class that imports some users, transforms the users information and then inserts it into a database. I have on file ImportEmployees that has a method which calls 2 other classes to import data. One class gets some users matching certain criteria and the other class handles the remaining users. These two classes do exactly the same thing except for one method. That one method is what extracts the users data correctly so there are 2 additional, different classes that it will call. What is a good design pattern that handles this well, or a better way to handle this to remove code duplication.

Here is the entry point class ImportEmployees: full class here

public function handle() { handle(new ImportNapaUsers($this->file)); handle(new ImportNonNapaUsers($this->file)); }

And the ImportNapaUsers class: full class here

private function extractUsers() { $users = new ExtractNapaUsers($this->users); $this->napaUsers = $users->getUsers(); }

And the ImportNonNapaUsers class: full class here

private function extractUsers() { $users = new ExtractNonNapaUsers($this->users); $this->napaUsers = $users->getUsers(); }

$this->users is the exact same in both of these. Also, both classes are identical except for the extractUsers method.

identical nope new ExtractNapaUsers is not new ExtractNonNapaUsers methods are not the same. Is that your code on git? And if not: Why do you want to change this? — JustOnUnderMillions
– JustOnUnderMillions, Commented Jan 20, 2017 at 17:08
Ignore my first comment. good design pattern dependency injection is the first to look for. Your handle() method is really against good oop. prevent to use new in a class method — JustOnUnderMillions
– JustOnUnderMillions, Commented Jan 20, 2017 at 17:12

Jeff · Accepted Answer · 2017-01-20 17:24:06Z

I would not overcomplicate this with design patterns. What you need is inheritance and abstract classes.

Create an abstract class ImportSomeUsers with an abstract method extractUsers() like this

abstract class ImportSomeUsers { ... abstract protected function extractUsers(); }

Then define your classes by extending the ImportSomeUsers class:

class ImportNapaUsers extends ImportSomeUsers { protected function extractUsers() { $users = new ExtractNapaUsers($this->users); $this->napaUsers = $users->getUsers(); } } class ImportNonNapaUsers extends ImportSomeUsers { protected function extractUsers() { $users = new ExtractNonNapaUsers($this->users); $this->napaUsers = $users->getUsers(); } }

@JustOnUnderMillions That definitely cuts down on duplication. I really appreciate your thoughts on DI. That is something that I didn't think about.

Community · Accepted Answer · 2017-05-23 11:52:56Z

I try to elaborate this:

 public function handle() { handle(new ImportNapaUsers($this->file)); handle(new ImportNonNapaUsers($this->file)); }

You have an class method that itself calls an global function, fully aginst oop
Then you staticly call this function with fixed use of new XYZ, also against oop
Then your public handle() has no return value, how do you note if something goes wrong?
Also your handle() function seems to returning nothing

And:

private function extractUsers() { $users = new ExtractNonNapaUsers($this->users); $this->napaUsers = $users->getUsers(); }

since the method is private this would be better:

$this->extractUsers(new ExtractNonNapaUsers($this->users));

And the method becomes

private function extractUsers($users) { $this->napaUsers = $users->getUsers(); }

It is a very complex topic, you should read more about it:

https://de.wikipedia.org/wiki/Dependency_Injection

Object Oriented PHP Best Practices

:)

tayfur · Accepted Answer · 2021-09-21 15:57:42Z

You also may use traits, something like:

namespace Traits; trait UserExtractor { private function extractUsers() { $users = new ExtractNapaUsers($this->users); $this->napaUsers = $users->getUsers(); } }

then ImportNapaUsers and ImportNonNapaUsers could use same method like:

use Traits\UserExtractor; class ImportNapaUsers // (class ImportNonNapaUsers) { use UserExtractor; //... }

Collectives™ on Stack Overflow

Refactoring PHP file to remove code duplication

3 Answers 3

2 Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Linked

Related