Как правильнее с точки зрения ООП?

[Lexey]

Как правильнее с точки зрения ООП?

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

1). Передать идентификатор атрибуту объекта и затем, вызвав соответствующий метод, удалить сообщение? Т. е. код примерно такой:

$gb->setMessageId($id);
$gb->deleteMessage();

1). Или предусмотреть и вызвать несколько другой метод, которому нужно передать идентификатор? Т. е. код примерно такой:

$gb->deleteMessage($id);

Какой вариант правильнее с точки зрения ООП? А какой вариант лучше использовать?

 

[AmdY]

.

 

[korchasa]

$gb->deleteMessage($id);

А если в API книги операции совершаются не только по id, то можно и более четко назвать:

$gb->deleteMessageById($id);

 

[Lexey]

Спасибо за ответы!
Я правильно понял, что если имеется хоть малейшая возможность не загонять предварительно какие-либо параметры в атрибуты объекта, а передавать их непосредственно при вызове "полезного" метода, то надо этой возможностью пользоваться? То есть, к примеру, добавление сообщения может выглядеть так:

$gb->addNewMassage($author, $text, $date);

?

 

[zerkms]

Lexey
и ещё неплохо было бы, если бы этот метод возвращал объект типа message

 

[Lexey]

Спасибо всем за ответы!

 

[Alexandre]

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

$gb->setMessageId($id);
$gb->deleteMessage();

проектируем из принципа: API должно быть разработано так, чтоб пользователь не мог допустить случайную ошибку. Можно разработать так:

$gb->deleteCurentMessage();
$gb->deleteMessageById($id);

Но если взглянуть глубже: то по названию или логике строка: $gb->deleteCurentMessage() - не соответствует модели. нарушается принцип Персональной ответственности
класс $db должен отвечать только за взаимодействие с БД, да и при таком подходе он превратится в МегаКласс, что уже дурно попахивает ...
необходим класс Message, который отвечает за модель Сообщений.
я бы сделал так:

 $msg = new Message( $db , ...);
$msg->setId( $id);
$msg->setParam( ....);
$msg->Add( ...);
$msg->deleteById($id);
$msg->deleteCurent();

 

[Angerslave]

Alexandre
А я бы предпочёл так:

<?php

$message = new Message($id);// if exists - get data from db, else - just create object
$message->setText($text);
$message->setAuthor($user1);
$message->save();//save in DB
$message->delete();

А всю эту буйню с суперобъектами - я не понимаю, чем это отличается от статических методов класса и т.п. функциональщины.

-~{}~ 15.11.08 15:24:

UPD: если в конструкторе объявить $id = null, то можно будет отслеживать создаётся ли новое сообщение или берётся имеющееся из базы.

 

[AmdY]

согласен мыслью о суперобъекте
тогда для класса GuestBook

public function getMessage($id) {/** return GuestBook_Message */}
public function deleteMessage($id) {
return $this->getMessage($id)->delete();
}

 

[korchasa]

Автор оригинала: Alexandre

$gb->setMessageId($id);
$gb->deleteMessage();

проектируем из принципа: API должно быть разработано так, чтоб пользователь не мог допустить случайную ошибку.

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

Автор оригинала: Alexandre
Но если взглянуть глубже: то по названию или логике строка: $gb->deleteCurentMessage() - не соответствует модели. нарушается принцип Персональной ответственности
класс $db должен отвечать только за взаимодействие с БД, да и при таком подходе он превратится в МегаКласс, что уже дурно попахивает ...

Согласен, но, ИМХО, код проще менять маленькими шагами, а не маленькими "революциями". И если ТС еще сомневается в названии методов, то шажки должны быть совсем мелкими. Ввели метод, протестировали, "увидели" новый класс, переделали реализацию deleteMessageById, протестировали.

А дальше еще удобно было бы и factory-метод сделать

class GuestBook {
...
  function createMessageForThisGuestBook()
  {
     $message = new GuestBookMessage();
     $message->setGuestBook($this);
      return $message;
   }
}

 

[AmdY]

$message->setGuestBook($this); - это неприемлемо.
в таком случае правильнее $this->addMessage($message)
это потребует правки в одном классе, вместо 2-х

 

[Alexandre]

Alexandre
А я бы предпочёл так:

не спорю с Титанами ООП...
а к БД - где привязывать будешь?

-~{}~ 15.11.08 16:25:

роектируем из принципа: API должно быть разработано так, чтоб пользователь не мог допустить случайную ошибку.
Вот как раз в этом случае легче совершить ошибку, ибо строки две, а не одна, и они могут разойтись по коду.

пусть они расходятся по коду....
$id - у нас сохраняется как элемент класса DB (хотя я указат? что это не правильно....)
по этому мы работаем с текущим id...
соответственно по этому и предложено было два разных метода

$msg->deleteById($id);
$msg->deleteCurent();

используем где надо deleteCurent(), а если id меняется, то deleteById($id)...

 

[korchasa]

Автор оригинала: AmdY
$message->setGuestBook($this); - это неприемлемо.
в таком случае правильнее $this->addMessage($message)
это потребует правки в одном классе, вместо 2-х

Очень глубокий вопрос. Можно похоливарить на несколько страниц, но давай не будем. Мне просто так проще, кому-то по-другому. Может лучше тогда $this->getMessages()->addMessage($message)? Так хотя бы коллекцию можно будет реюзать.

пусть они расходятся по коду....
$id - у нас сохраняется как элемент класса DB (хотя я указат? что это не правильно....)
по этому мы работаем с текущим id...
соответственно по этому и предложено было два разных метода

используем где надо deleteCurent(), а если id меняется, то deleteById($id)...

Для меня наличие current это признак итератора, тогда уж надо идти до конца - setCurrentMessage($id), resetMessages(), getNextMessage(), deleteCurrentMessage(). Потом мы хотим сделать похожую штуку для другого объекта, и приходим к $gd->getMessages()->setCurrent($id)->delete(). Так и рождаются ORM'ы))

А проще все таки сначала использовать TableGateway.

 

[Angerslave]

Alexandre
Да какой там титан, просто взглянув на код, а потом на топик, что-то не увидел там реального ООП.

$msg->deleteById($id);
$msg->deleteCurent();

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

 

[korchasa]

Автор оригинала: Angerslave
Эм, а в абстракциях это как? Один объект может сделать харакири, это понятно, но что он может и другие объекты убивать - это уже перебор, имхо.

А не было тут другого объекта. Точнее бизнес-сущность может и есть, но в коде отдельного класса не реализована. Он был реализован в gb.

Или попытка уместить в одном классе и суперобъект(удалять по ID) и возможность использовать его как минимальную абстракцию(просто сообщение).

Это попытка уместить в одном классе 3 разных сущности - книгу, коллекцию сообщений, сообщение. Чем это плохо, кроме потенциальной сложности последующего разделения (из-за зависимостей от API)?

 

[Alexandre]

Да какой там титан, просто взглянув на код, а потом на топик, что-то не увидел там реального ООП

Angerslave вот и я не увидел и предложил более менее приемлемое решение решение, а получих холивар...
все это так относительно, одним нравится так, другим эдак... Вот и я стараюсь не спорить с Титанами...
не прав, ну извините... возможно чье-то решение лучше. Я, себя мега ООПшником не считаю, возможно где-то не прав

 

[AmdY]

$this->getMessages()->addMessage($message)
почему ты так упорно пытаешься привязать сообщение к гостевой книге, это лишнее связывание. сообщения могут спокойно существовать и не зная о классе гостевой книги, это нужно гостевой книге, пускай она этим и занимается

 

[Angerslave]

korchasa
В том-то и дело, что если так подходить, то всё можно в класс Object пихать - совместимо и ладно Но лучше, имхо, чётко разделять обязанности классов.
AmdY
+1, в гестбуке, видимо, даже категорий нет, можно GuestBook заменить на абстрактный MessageList без потери смысла

 

[korchasa]

Автор оригинала: Amdy
$this->getMessages()->addMessage($message)
почему ты так упорно пытаешься привязать сообщение к гостевой книге, это лишнее связывание.

В приведенном коде сообщение не связано с книгой. Тут связь книга => что-то => сообщение.

Автор оригинала: Amdy
сообщения могут спокойно существовать и не зная о классе гостевой книги, это нужно гостевой книге, пускай она этим и занимается

Ну да, могут. Вводим отдельную таблицу для связи и понеслась.

Автор оригинала: Angerslave
korchasa
В том-то и дело, что если так подходить, то всё можно в класс Object пихать - совместимо и ладно Но лучше, имхо, чётко разделять обязанности классов.

А где тут НЕ четкое разделение? Класса GuestBookMessage НЕТ. Тогда GuestBook реализует работу с базой данных, и сообщениями. Мы разговариваем о двух разных случаях:
- у нас есть GuestBookMessage. Тогда имеем:
GuestBook - работает со своими данными, знает как получить все свои сообщения
GuestBookMessage - работает со своими данными
какую-то коллекцию сообщений, например массив

- мы не хотим вводить GuestBookMessage, т.к. отдельно он у нас нигде не фигурирует.

 

[Angerslave]

korchasa
В том-то и дело, что первоначально поставленный вопрос "Какой вариант правильнее с точки зрения ООП? А какой вариант лучше использовать?", имхо, подразумевает ответ "никакой - это всё функциональный стиль". А уже дальше плясать - что важнее - абстрагирование сообщения от контейнера или гостевая книга как просто контейнер каких-то данных. И тут мы пытаемся сесть одной задницей на два стула - и представить гостевуху как контейнер, и представить сообщение как очень независимый объект. По-моему, эти цели плохо уживаются вместе Разве что юзать функциональный стиль, как в первом посте.