Является ли это правильным объектно-ориентированным программированием в php?

Ответ 1

Ради Сохраняя это просто, я бы сказал, все в порядке. Это не слишком правильное ООП, хотя и не очень понятно понять код. Имея рабочий код лучше, чем отсутствие кода, хотя.

Пропустите свой код:

1 class Greeting {
2
3    public $greet = array('Hi','Hello', 'Howzit', 'Ola', 'Whats up');
4
5    function __construct($name) {
6        $this->name = $name;
7        shuffle($this->greet);
8    }
9 }

Строка 1: говорит, что этот класс представляет концепцию приветствия. Что такое Приветствие? Я бы сказал что-то вроде "Hello John" или "Hi John" или "Howdy John" - это приветствие. И на самом деле вы, кажется, согласны, потому что в...

Строка 3:... у вас есть список похожих приветствий, просто без имени. Но это свойство требует ответа на вопрос, почему ваш класс называется Greeting, когда он действительно инкапсулирует несколько приветствий. Разве класс не следует называть "Приветствия" (помните о множественном числе)?

Строка 3. Именование свойства "приветствие" тоже не слишком хорошая идея. Это свойство, поэтому не давайте ему имя глагола. Глаголы для методов.

Линия 3. Хотя есть люди, которые расскажут вам разные, что делает публичную собственность редко хорошей идеей. Свойства - это внутреннее состояние, и это состояние не должно быть доступно напрямую, но только с помощью методов.

Строка 5. Затем ваш конструктор сообщает мне, что приветствие должно иметь имя. Если бы я уже не смотрел исходный код, я бы ошибочно предполагал, что это имя Приветствия. Но вы действительно имеете в виду имя человека. Аргумент должен отражать это и называться чем-то более показательным, например $greetedPersonsName.

Строка 6. Назначение свойств "на лету" - это boo boo. Если я посмотрю на определение класса, я хочу сразу увидеть свойства. Обнаружение их внутри некоторого метода затрудняет понимание кода. Это также не будет воспринято при создании документов API. Избегайте этого.

Линия 7: shuffle - еще одна неожиданная вещь. Это неочевидный побочный эффект. Если бы я хотел создать новое приветствие, я ожидаю, что приветствия появятся в том порядке, в котором они перечислены. Это против Принципа наименьшего Astonishement, чтобы перетасовать их в ctor. Перетасовка должна происходить из открытого метода, который ничего не делает, кроме перетасовки, например.

public function shuffleGreetings()
{
    shuffle($this->greetings);
}

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

public function getGreeting()
{
    return $this->_greetings[0] . ' ' . $this->name;
}

Это лучше, чем делать

echo $hi->greet[1] .' '. $hi->name;

поскольку он скрывает детали реализации. Мне не нужно знать, что объект Приветствие имеет множество возможных приветствий. Я просто хочу получить приветствие в сочетании с заданным именем. Он все еще далек от совершенства, хотя, потому что вы все равно будете использовать его, как

$hi = new Greeting('John'); // A Greeting named John? Why $hi then?
$hi->shuffleGreetings();    // Shuffling Greetings in a Greeting?
echo $hi->getGreeting();    // Why is it "Hello John" all of a sudden?

Как вы можете видеть, API по-прежнему в значительной степени полон WTF. Разработчику все равно придется искать исходный код, чтобы понять, что происходит.

Подробнее о побочных эффектах

Хотя может возникнуть соблазн поставить shuffle на getGreeting, вы не должны этого делать. Метод должен возвращать то же самое для одного и того же ввода. Когда я вызываю getGreeting дважды подряд, я могу ожидать, что он вернет тот же результат. Вы ожидали бы, что 1 + 1 будет возвращать 2 всегда, поэтому убедитесь, что ваши методы тоже работают.

Аналогично, если вы хотите, чтобы один метод возвращал случайный элемент из свойства greetings, не перетасовывайте массив приветствий. Если вместо этого вы будете использовать метод shuffle, вы также измените свойство greetings. Это пульсирует по любой функции, считанной из свойства, например. когда вы делаете

public function getRandomGreeting()
{
    $this->shuffleGreetings();
    return $this->getGreeting();
}

разработчик испытает что-то вроде этого:

$hi = new Greeting('John');
$hi->shuffleGreetings();
echo $hi->getGreeting();       // for example "Hello John"
echo $hi->getRandomGreeting(); // for example "Hi John"
echo $hi->getGreeting();       // for example "Howdy John" <-- WTF!!

Используйте реализацию, которая не изменяет свойство, например.

public function getRandomGreeting()
{
    $randomKey = array_rand($this->greetings);
    return $this->greetings[$randomKey] . ' ' . $this->name;
}

Это не имеет побочных эффектов:

$hi = new Greeting('John');
$hi->shuffleGreetings();
echo $hi->getGreeting();       // for example "Hello John"
echo $hi->getRandomGreeting(); // for example "Hi John"   
echo $hi->getGreeting();       // still "Hello John". Expected!

API все еще далеко не хорош. Если я думаю о свойствах Приветствия, я просто не думаю "Имя человека". Просто "Привет" или "Привет" по-прежнему является действительным приветствием. Он не требует имени. Как насчет

public function greetPerson($personName)
{
    return $this->getGreeting() . ' ' . $personName;
}

а затем мы можем сделать

$hi = new Greeting;
$hi->shuffleGreetings();
echo $hi->greetPerson('John');

И, наконец, скрыть, что наше приветствие содержит массив, который нужно перетасовать, переместите наш метод shuffleGreetings обратно в ctor и переименуйте класс в RandomGreeting.

class RandomGreeting …

    public function __construct()
    {
        $this->shuffleGreetings();
    }

Это может показаться вам неразумным, потому что я сказал вам не перетасовывать в ctor. Но с классом, переименованным в RandomGreeting, гораздо больше можно ожидать, что что-то происходит за кулисами. Нам просто не нужно знать, что именно. Чтобы отразить это, мы также должны сделать метод shuffleGreetings защищенным сейчас. Мы просто полностью скрываем его от открытого интерфейса. Теперь наш код читается следующим образом:

$hi = new RandomGreeting;
echo $hi->greetPerson('John'); // ex "Howdy John"

Это не дает вам никаких WTF, потому что ваш код четко сообщает, что вы получите случайное приветствие. Имя класса ясно сообщает, что он делает.

Теперь это немного лучше, и мы можем кончить здесь, но можно все же утверждать, что приветствие не должно приветствовать на нем, а скорее нечто, что делает Личность вместо этого.

Улучшение его

Наши выводы должны привести нас к выводу, что приветствие скорее должно быть немым типом, инкапсулирующим приветственное послание, и ничего больше. Все, что он должен сделать, это вернуть это сообщение. Поскольку у Greeting нет никакого реального поведения рядом с сохранением строки сообщения, проще всего было бы создать объект Value, например. объект, который равен другому объекту по значению свойства:

class Greeting
{
    protected $value;

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

    public function getValue()
    {
        return $this->value;
    }
}

Другой способ сделать это - сделать различные доступные приветствия отдельными типами. Этого очень мало, потому что у ваших объектов нет поведения. Вам нужно только это, если вы хотите воспользоваться Полиморфизмом. Тем не менее, наличие конкретных подтипов делает несколько дополнительных вещей, которые следует рассмотреть позже, поэтому давайте предположим, что нам это нужно.

Правильный способ сделать это в ООП будет определить интерфейс

interface Greeting
{
    public function getGreeting();
}

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

abstract class GreetingType implements Greeting
{
    protected $greeting;
    public function getGreeting()
    {
        return $this->greeting;
    }
}

Когда существует абстрактный класс, также должны быть конкретные классы, полученные из абстрактного класса. Поэтому позвольте использовать Inheritance, чтобы определить наши конкретные типы приветствий:

class HiGreeting extends GreetingType
{
    protected $greeting = 'Hi';
}

class HelloGreeting extends GreetingType
{
    protected $greeting = 'Hello';
}

class HowdyGreeting extends GreetingType
{
    protected $greeting = 'Howdy';
}

Не обязательно иметь интерфейс и абстрактный интерфейс, реализующий интерфейс. Мы могли бы сделать наши конкретные приветствия не простирающимися от GreetingType. Но если бы мы только переопределили метод getGreeting во всех различных классах приветствия, мы бы дублировали код и были гораздо более склонны к внедрению ошибок, и если нам когда-либо понадобится что-то изменить, нам придется прикоснуться ко всем этим классам. С GreetingType все централизовано.

И наоборот. И наоборот. Вам не обязательно нужен интерфейс. Мы могли бы использовать только абстрактный тип. Но тогда мы ограничились GreetingType, тогда как с интерфейсом мы могли бы добавить новые типы с гораздо большей легкостью. Я признаю, что сейчас не могу придумать, так что это, вероятно, YAGNI. Но это так мало, чтобы добавить, что мы можем просто сохранить его сейчас.

Мы также добавим Null Object, который возвращает пустую строку. Подробнее об этом позже.

class NullGreeting extends GreetingType
{
    protected $greeting = '';
}

Создание объекта

Потому что я не хочу засорять new classname все мои классы потребления и ввести coupling, я буду использовать простой Factory вместо создания объекта капсулы:

class GreetingFactory
{
    public function createGreeting($typeName = NULL)
    {
        switch(strtolower($typeName)) {
            case 'hi':    return new HiGreeting;
            case 'howdy': return new HowdyGreeting;
            case 'hello': return new HelloGreeting;
            default:      return new NullGreeting;
        }
    }
}

Factory является одним из немногих фрагментов кода, в котором вы действительно можете использовать swich/case, не проверяя, можете ли вы заменить условный на полиморфизм.

Ответственность

С созданием объекта с пути мы можем, наконец, начать добавлять наш класс Greetings:

class Greetings
{
    protected $greetings;
    protected $nullGreeting;
    public function __construct(NullGreeting $nullGreeting)
    {
        $this->greetings = new ArrayObject;
        $this->nullGreeting = $nullGreeting;
    }
    public function addGreeting(Greeting $greetingToAdd)
    {
        $this->greetings->append($greetingToAdd);
    }
    public function getRandomGreeting()
    {
        if ($this->hasGreetings()) {
            return $this->_getRandomGreeting();
        } else {
            return $this->nullGreeting;
        }
    }
    public function hasGreetings()
    {
        return count($this->greetings);
    }
    protected function _getRandomGreeting()
    {
        return $this->greetings->offsetGet(
            rand(0, $this->greetings->count() - 1)
        );
    }
}

Как вы можете видеть, Приветствия на самом деле просто оболочка для ArrayObject. Он гарантирует, что мы не можем добавить ничего, кроме объектов, реализующих интерфейс Greeting для коллекции. И это также позволяет нам выбрать случайное приветствие из коллекции. Он также гарантирует, что вы всегда получаете приветствие от вызова getRandomGreeting, возвращая NullGreeting. Это потрясающе, потому что без этого вам придется делать

$greeting = $greetings->getRandomGreeting();
if(NULL !== $greeting) {
    echo $greeting->getMessage();
}

чтобы избежать "Fatal Error: Trying to call method on non-object", когда метод getRandomGreeting не возвратил объект Greeting (когда еще нет приветствия в классе Приветствия).

В классе нет другой ответственности. Если вы не уверены в том, что ваш класс слишком много или имеет методы, которые лучше должны быть на каком-то другом объекте, посмотрите на методы этого класса. Они работают с свойством этого класса? Если нет, вы должны, вероятно, переместите этот метод.

Выполнение

Теперь, чтобы, наконец, использовать весь этот код, мы добавим теперь класс Person. Поскольку мы хотим удостовериться, что можем называть метод getName, мы создаем интерфейс, прежде чем делать это

interface Named
{
    public function getName();
}

Мы могли бы назвать этот интерфейс IPerson или что-то еще, но у него есть только один метод getName, и наиболее подходящее имя называется Named, потому что любой класс, реализующий этот интерфейс, является именованным, включая, но не ограничиваясь, наш класс Person:

class Person implements Named
{
    protected $name;
    protected $greeting;
    public function __construct($name, Greeting $greeting)
    {
        $this->name = $name;
        $this->greeting = $greeting;
    }
    public function getName()
    {
        return $this->name;
    }
    public function greet(Named $greetable)
    {
        return trim(sprintf(
            '%s %s',
            $this->greeting->getGreeting(),
            $greetable->getName()
        ));
    }
}

У нашего лица есть требуемое имя, и мы требуем, чтобы он получил Приветствие. Все, что он может сделать, кроме того, чтобы вернуть это имя, приветствует другую Именованную вещь, вероятно, другого человека. И что это.

Теперь все вместе:

$greetings->addGreeting($greetingsFactory->createGreeting('Hi'));
$greetings->addGreeting($greetingsFactory->createGreeting('Howdy'));
$greetings->addGreeting($greetingsFactory->createGreeting('Hello'));
$john = new Person('John Doe', $greetings->getRandomGreeting());
$jane = new Person('Jane Doe', $greetings->getRandomGreeting());

echo $john->greet($jane), 
     PHP_EOL, 
     $jane->greet($john);

Live Demo на Codepad

Согласен, что довольно много кода для очень простой вещи. Некоторые вызовут это overengineered. Но вы спросили о правильном ООП, и, хотя я уверен, что есть еще возможности для улучшения, он вполне подходит и SOLID в моей книге. И это легко поддерживать сейчас, потому что обязанности ближе к тому, где они должны быть.

Ответ 2

Хм, две вещи:

• перетасовка этого массива и доступ к нему извне, а внешний вызов зависит от перетасованного массива, не кажется правильным. Было бы лучше иметь метод, который возвращает результат перетасовки массива и, возможно, также имя сразу.

• Было бы хорошим стилем объявить все свойства объекта заранее, чтобы его можно было документировать позже. Поэтому, если вы используете $this->name, вы должны объявить его.

Обе реализованные точки могут выглядеть так:

class greeting 
 {
 protected $greet = array('Hi','Hello', 'Howzit', 'Ola', 'Whats up');
 protected $name = null;

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

 public function greet()
 { 
  shuffle($this->greet);
  return $this->greet[1]." ".$this->name;
 }
}

Ответ 3

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

True OOP - это когда вы разделяете отдельные объекты вашего приложения на объекты/зависимости.

например, простой веб-сайт будет включать следующее:

• База данных
• Обработка ошибок
• Сессия
• Безопасность

они называются сущностями и не должны взаимодействовать напрямую друг с другом, поэтому их разделять.

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

Глядя на вашу попытку в классе приветствия, я бы посмотрел на что-то вроде этого:

class User
{
    protected $data;

    public function __construct(array $user_data)
    {
        $this->data = $user_data;
    }

    public function getUsername()
    {
        return $this->data['username'];
    }
}

class Greeting
{
    private $message = "welcome to mysite %s";

    public function __construct(string $to_greet)
    {
        $this->message = sprintf($this->message,$to_greet);
    }

    public function getGreeting()
    {
        return $this->message;
    }
}

будет использовано следующее:

$User = new User(array(
    'id' => 22,
    'username' => 'Robert Pitt'
));

$Greeting = new Greeting($User->getUsername());

echo $Greeting->getGreeting();

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

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

если объекты связаны друг с другом так, что все они связаны друг с другом, вы можете сделать что-то вроде этого:

$User = new User(/*..From DB ..*/);
$Greeting = new Greeting($User);

echo $Greeting->getGreeting();

Ответ 4

Вероятно, было бы лучше иметь метод greet. Идея состоит в том, что пользователю не нужно знать, что $this- > greet является массивом.

Итак:

class Greeting { 
  protected $greet = array('Hi','Hello', 'Howzit', 'Ola', 'Whats up'); 
  protected $name='You';    

  function __construct($name) {
         $this->name = $name;         
  }

  function greet(){
    shuffle($this->greet);
    echo "{$this->greet[0]} {$this->name}!";
  }

}  
$hi = new Greeting('INSERTNAMEHERE');/*NAME OF PERSON GOES HERE*/ 
$hi->greet(); 

Ответ 5

Я должен не согласиться с [Изменить: Некоторые из] других ответов, которые вы получили. Создание greet private и добавление getter мало для увеличения инкапсуляции.

Независимо от этого, однако, я думаю, что это довольно плохой дизайн OO. Объект должен представлять что-то (осмелюсь сказать, некоторые фактические предметы)? ИМО, "объекты" здесь будут действительно людьми (или что-то еще), один из которых приветствует другого. Самым приветствием должно быть сообщение, переданное от одного из этих объектов другому. Мы могли бы написать класс приветствия, чтобы сохранить текст приветствия, но даже если мы делаем сам объект приветствия, он должен быть независимым от источника или цели приветствия.

Ответ 6

На самом деле это немного не по теме. Прежде всего, общедоступные переменные, как правило, плохие. Вы не должны получать доступ к переменным класса непосредственно из кода. Вы должны попытаться инкапсулировать функциональные возможности, предоставляемые классом, в сам класс.

Во-первых, позвольте мне показать вам, как хороший вид OOP-кода может выглядеть на основе вашего примера:

<?php

class greeting
{
    private static $greets = array('Hi','Hello', 'Howzit', 'Ola', 'Whats up');

    private $name;

    function __construct ($name)
    {
        $this->name = $name;
    }

    function doGreeting ()
    {
        $i = array_rand(self::$greets);
        return self::$greets[$i] . ' ' . $this->name;
    }
}

$hi = new greeting('INSERTNAMEHERE');/*NAME OF PERSON GOES HERE*/
echo $hi->doGreeting();

Теперь поговорим о различиях здесь.

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

Еще одна небольшая деталь заключается в том, что имя также объявлено там как private, так как все переменные класса должны быть приватными вообще для инкапсуляции. Если им необходимо получить доступ, это должно выполняться с помощью функций setter и getter, которые могут обеспечить дополнительную проверку значений, если это необходимо.

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

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

Помните инкапсуляцию и повторное использование. Это два важных момента ООП.

Ответ 7

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

Объект "лучше" будет выглядеть примерно так:

class greeting {
   protected $greet = array('Hi','Hello', 'Howzit', 'Ola', 'Whats up');
   protected $name;
   function __construct($name) {
       $this->name = $name;
   }
   public function greet() {
       return $this->greet[rand(0,5)] . ' ' . $this->name;
   }
}

$hi = new greeting('INSERTNAMEHERE');/*NAME OF PERSON GOES HERE*/
echo $hi->greet();

Теперь ваш объект greet полностью контролирует приветствие, и ваш код приложения не должен заботиться о том, как этот объект настроен, он просто должен заботиться о нескольких функциях, которые он может использовать для получения приветствия. Если вы хотите в какой-то мере интернационализировать или изменить приветствие, все, что вам нужно сделать, это изменить его на один объект.

Кроме того, просто отметить, что использование объектов не делает ваш код ООП. Он должен быть объектно ориентированным, что означает использование объектов для определения дискретных задач, которые необходимо выполнить вашему приложению, а не для того, чтобы каждая другая часть приложения имела полный доступ ко всем этим данным, чтобы делать то, что будет. Затем вы просто создаете структуры данных.

Ответ 8

это не совсем корректный OOP, потому что переменная $greet не инкапсулирована.

чтобы сделать его "более правильным" (что бы это ни значило), вам нужно сделать private $greet param и создать некоторые методы get и set для него (то же самое касается переменной $name), которая также должна объявляется до его использования).

в php метод get выглядит так:

если я хочу получить var:

$hi = new greeting('INSERTNAMEHERE');/*NAME OF PERSON GOES HERE*/
echo $hi->__get('greet') .' '. $hi->__get('name');

get создается внутри класса приветствия:

function __get($var){
    return $this->$var;
}

и установить одно и то же:

function __set($var, $val){
    $this->$var = $val;
}

Ответ 9

Я бы, вероятно, использовал getters/setters для $greet вместо того, чтобы публиковать его. Я не понимаю, почему это не будет считаться ООП.

http://en.wikipedia.org/wiki/Object-oriented_programming