Как провести рефракторинг различающихся сигнатур методов в потомках (php7)?

Question

[Andry]

php7 бросает warning (раньше был "незаметный" strict) сигнатура метода наследника не совпадает с родителем.

PHP Warning: Declaration of ns\Captcha::destroy($memberId) should be compatible with captcha\Obj::destroy()

Код наследника

public function destroy($memberId){
  parent::destroy();
  db::query("update member set ncaptcha = 0 where id = :memberId", $memberId);
}

Ситуация не единичная, таких методов полная кошелка. Посему ломаю голову, как правильней проводить рефракторинг подобного кода. Понимаю, что каждый случай уникальный, но все же. Кто какие варианты рефракторинга подскажет?

В голове вертятся варианты:
1. В потомке новый метод destroyByMember($memberId) , но тогда мы получаем в наследство "неправильный метод" destroy(). Получается его нужно переводить e родителя в protected и в потомках публиковать свои методы destroyBy... c нужной сигнатурой.

2. Прилепить в потомке отдельный сеттер setMember($memberId) и через него указывать memberId для переопреленного destroy()

Answer 1

[Boris Köln]

Вариант 1 (если сайтом уже пользуются).
Писать все warning в лог. Регулярно читать и исправлять.

Вариант 2 (если есть автотесты).
Выводить все warning на экран и исправлять.

Вариант 3.
Воспользоваться статическим анализатором кода. Например, https://plugins.jetbrains.com/plugin/7622-php-insp...

Comments

[Andry]

Вот я логи читаю и пытаюсь исправлять )

Вопрос в том и заключается - как рефракторить такого рода код?

[Boris Köln]

как рефракторить такого рода код?

Вариант 1.
В родителе: destroy($memberId)

Вариант 2.
В потомке: destroy($memberId = null)

Вариант 3.
В потомке: destroyByMember($memberId)

[Станислав Б]

Boris Korobkov,
"Функции, которые используют базовый тип, должны иметь возможность использовать подтипы базового типа, не зная об этом."(с)

а вообще не ясно что у вас там под destroy скрывается и как он используется, может вам там вообще наследование не нужно, а нужно пару интерфейсов объявить и разные реализации сделать, например, так что сложно что то посоветовать.

[Boris Köln]

"Функции, которые используют базовый тип, должны иметь возможность использовать подтипы базового типа, не зная об этом."(с)

Станислав Б, да. Именно поэтому и надо рефакторить, чтобы родителя и потомка сделать совместимыми (мои варианты 1 и 2).

может вам там вообще наследование не нужно, а нужно пару интерфейсов объявить и разные реализации сделать

мой вариант 3

[Andry]

Посидел за кодом и вижу, что общие рецепты с такой бедой не вырисовуются.

В описаном случае убрал метод в потомке и запрос к БД перенес в другое место.

[Boris Köln]

общие рецепты с такой бедой не вырисовуются

Вообще-то есть 3 вышеуказанных метода.

Answer 2

[Александр Талалаев]

Вы конечно мастер объяснений, к тому же сложно телепатически объять всю структуру приложения и понять что где происходит, что там у родителя, где и как используются/вызываются методы и тп.

Но забавнее всего видеть слово рефакторинг и отсутвие желания исправить ужас:

db::query("update member set ncaptcha = 0 where id = :memberId", $memberId);

, эх..

public function destroy(int $memberId){
//Что тут и зачем хз,  что там происходит ты не написал
parent::destroy();
try{
//Что такое db? PDO?
//Далее почти тупо копипаст из маст рид книги для новичоков Php right way
$stmt = db::prepare('update member set ncaptcha = 0 where id = :memberId');
$stmt->bindParam(':memberId', $memberId, PDO::PARAM_INT); 

if(!$stmt->execute()){
throw new Exception($stmt->errorInfo()[2]);
}

}
 catch (Exceprion $e ) {
throw $e;
}
//А и мы что-нибудь возвращаем? Если да, то выше нужно указать и тип возвращаемого значения у метода
}

ну и try нужно повесить на вызов метода:

//Где-то в далеких недрах кодинга
try {
$tvoyPeremenayaClass->destroy($memberId);
}
catch (Exception $e)
{
echo 'Прости Юра, мы все просрали ...' . $e->getMessage();
}

И это только примерно я написал и это только начало. Вообще нужно начинать с того что поставить php7.1

Comments

[Andry]

Спасибо за ваше внимание к скромному методу db::query(). Поверьте, под этим скромным именем спрятано достаточно )

Я же прошу обратить внимание на рефракторинг определенного типа проблемного кода. Потмоу и привел лишь саму соль. Метод db::query() остался лишь как объяснение, для чего вообще метод переопределен с новой сигнатурой.

Я не жду точного и верного ответа. Мне интересны методики/способы подобного рода рефракторинга.

____
P.S.
PHP Version 7.1.11

[Александр Талалаев]

Ну пойми, я же написал что давать советы, не знаю структуру невозможно. Ты даже UML диаграмму классов не привел и тд. Какие советы можно дать? Просто советы как убрать симптомы, а может лучше перерассмотреть общую структуру.

Answer 3

[Night]

Посему ломаю голову, как правильней проводить рефракторинг подобного кода.

Просто проводить рефакторинг, чтобы таких нюансов не было. Приводите всех предков к единой сигнатуре.