В конце концов, я получил эту функцию. Я не знаю, нормально это или нет.
function user_registration($user_name, $user_email, $user_pass, $address,
$city, $postalcode, $country, $phone, $mobilephone)
Как и почему я могу улучшить это?
Вы можете либо передать массив со всеми связанными друг с другом параметрами, либо просто создать класс "Пользователь" и добавить все свойства через сеттеры и в конце завершить проверку с помощью специального метода:
class User {
public function setName($name) {
$this->name = $name;
}
[...]
public function register() {
//Validate input
if (empty($this->name))
$this->errors[] = "ERROR, Username must not be emtpy";
//Add the user to the database
//Your SQL query
return empty($this->errors);
}
}
$user = new User();
$user->setName("Peter");
$success = $user->register();
if (!$success)
echo "ERRORS OCCURED: ".print_r($user->errors, true);
Решение будет состоять только в том, чтобы иметь только один параметр, который может содержать несколько фрагментов данных - как массив.
Ваша функция может быть определена следующим образом:
function user_registration(array $data) {
// work with $data['name']
// and $data['email']
// ...
}
И вы бы назвали это следующим образом:
user_registration(array(
'name' => 'blah',
'email' => '[email protected]',
'pass' => '123456',
// and so on
));
Хорошие вещи:
Не так уж плохо:
Я лично не думаю, что у него слишком много параметров. От взгляда на определение функций ясно, что вы требуете в качестве входных данных, которые не были бы столь очевидными, если вызывается с массивом.
"Если это не сломано, не исправить это!"
Как общее правило (не как правило стойкости), в любое время вы должны спросить: "У этой функции слишком много параметров?" - Да. Ваша интуиция говорит вам кое-что, что ваш мозг еще не смог выработать.
В этом конкретном случае первое, что приходит в голову, это то, что сначала нужно проверить ваш credrs пользователя (существует ли уже имя пользователя?), и ваши данные пользователя должны добавляться отдельно, возможно, используя объект или массив.
Один из способов - передать массив как параметр этой функции и поместить всю информацию в этот массив:
function user_registration(array $user_info)
{
// process $user_info;
}
Когда вы смотрите на имена аргументов, вы не можете не заметить, что их можно сгруппировать в три разные группы:
User Data: $user_name, $user_pass
Address Data: $address, $city, $postalcode, $country
Contact Data: $user_email, $phone, $mobilephone
Следовательно, вы можете применить Ввести объект параметров:
Часто вы видите определенную группу параметров, которые, как правило, передаются вместе. Несколько методов могут использовать эту группу либо на одном классе, либо в нескольких классах. Такая группа классов представляет собой совокупность данных и может быть заменена объектом, который несет все эти данные. Стоит превратить эти параметры в объекты только для группировки данных. Этот рефакторинг полезен, потому что он уменьшает размер списков параметров, и длинные списки параметров трудно понять. Определенные аксессоры на новом объекте также делают код более последовательным, что опять упрощает его понимание и изменение.
Если вы не хотите делать ООП, вы также можете группировать аргументы в массивы, но тогда вы потеряете все преимущества типа. Я просто предполагаю, что вы не против использования объектов. Итак, после применения Refactoring вы получите
function user_registration(User $user, Address $address, Contact $contact)
Глядя на этот список параметров, вы должны заметить, что адрес и контакт, вероятно, принадлежат пользователю в первую очередь, поэтому вы можете рассмотреть возможность замены сигнатуры функции только на
function user_registration(User $user)
а затем вызовите его следующим образом:
$user = new User('johndoe', 'secretsauce');
$user->setAddress(new Address('Doe Street', 'Doe Town', 12345, 'Neverland'));
$user->setContact('[email protected]', '+123 12345', '+123 54321');
user_registration($user);
Возможно, мы могли бы ввести имя пользователя и пароль в объект Credentials, а затем просто сделать
user_registration(new User($credentials, $address, $contact));
Требуя данных в ctor, мы убеждаемся, что у новых зарегистрированных пользователей есть все эти данные. Мы могли бы утверждать, нужен ли нам адрес и контакт для регистрации пользователей, поэтому "Утилита установки" может быть достаточно хороша здесь:
$user = new User(new Credentials('johndoe', 'secretsauce'));
$user->setAddress(new Address('Doe Street', 'Doe Town', 12345, 'Neverland'));
$user->setContact(new Contact('[email protected]', '+123 12345', '+123 54321'));
user_registration($user);
Тем не менее, user_registration как отдельная функция в глобальной области действия неуместна. По Принцип экспертной оценки GRASP, методы должны быть на объектах, которые имеют наибольшую информацию для выполнения ответственности. Это улучшает Cohesion и уменьшает Coupling. Другими словами:
$user = new User($credentials);
$user->setAddress($address);
$user->setContact($contact);
$user->register();
Одна проблема с классом User теперь состоит в том, что он содержит пароль. Пароль необходим только для аутентификации пользователя с помощью службы проверки подлинности. Мы можем спорить о имени пользователя, но пароль определенно не должен быть частью объекта User вообще. Поэтому вы должны сделать что-то вроде
$user = new User;
$user->setAddress($address);
$user->setContact($contact);
$user->register($credentials);
и когда вызывается register(), он будет использовать учетные данные только для делегирования вставки нового пользователя в хранилище пользователя. Но он не сохранит их в фактическом экземпляре пользователя.
Наконец, вы можете захотеть добавить Простой шаблон Factory или Builder, чтобы инкапсулировать создание пользователя, чтобы упростить aggregation различных случаев. Или вы можете ввести шаблон репозитория и переместить метод register() там. Однако это не подлежит рассмотрению.
Я делаю массив ключей вроде
$fields = array('field1', 'field2');
function register (array $values, array $keys)
{
$data = array();
foreach ($keys as $one)
{
if (isset($values[$one])) $data[$one] = $values[$one];
}
// or you can use array functions like array_flip and after - array intersect
}
Функция (метод) без каких-либо параметров лучше всего. Функция с одним пареметром лучше, чем функция с двумя параметрами. Функция с 2 параметрами лучше, чем функция с 3 параметрами и т.д.
@Florianh дал идеальное решение, как ваш код может быть улучшен. С этим комментарием я хотел бы подробно рассказать о "почему" с точки зрения дизайна системы.
В объектно-ориентированной перспективе другие объекты должны иметь возможность манипулировать атрибутами. Вот почему атрибуты никогда не должны определяться как "публичные". Они должны быть "private" например:
private var $name;
Причина этого в том, что другие объекты будут манипулировать этой переменной, правильная работа объекта находится под угрозой. С другой стороны, методы могут быть определены публично:
public function register()
Соответственно, манипулирование атрибутами будет происходить с помощью соответствующих методов. Метод также может использоваться для оценки правильности операций над атрибутами.
Есть две операции, которые могут произойти: чтение текущего значения атрибута с помощью Получить методы и сохранение нового значения атрибута с помощью Установить методы.
Хорошей практикой было бы реализовать метод get для каждого атрибута класса. Каждый атрибут, который может быть изменен, также должен иметь соответствующий метод набора.
Иногда лучше не реализовывать метод get/set (например: showData()). Это связано с тем, что использование геттеров и сеттеров в определенном классе может привести к снижению производительности. Однако это означает, что при изменении или внедрении класса необходимо быть осторожным при попытке сохранить ложную информацию и, как результат, подвергая целостность класса риску.
Теперь рассмотрим тот факт, что вы решили использовать только один телефонный номер вместо номера телефона и мобильного телефона. Когда мобильный номер становится устаревшим, ваша основная программа по-прежнему остается неизменной. Все, что вам нужно сделать, это изменить/удалить один метод. Преимуществом использования геттеров и сеттеров является увеличение адаптивности и ремонтопригодности.
Я бы сделал это таким образом
fields=explode(",","name,surname,lastname,street,city,region,zip,country");
user_registration($fields);
Потому что я уверен, что эти переменные, исходящие из $_POST