Какой дизайн лучше?

Question

У меня есть следующий класс:

class User {

  public function setName($value) { ... }
  public function setEmailAddress($value) { ... }
  public function setUsername($value) { ... }
  public function getName() { ... }
  public function getEmailAddress() { ... }
  public function getUsername() { ... }

  public function isGroupAdministrator($groupId) { ... }
  public function isMemberOfGroup($groupId) { ... }
  public function isSiteAdministrator() { ... }
  public function isRoot() { ... }
  public function hasModulePermission($moduleId, $recordId, $permissionCode) { ... }
  public function hasGroupPermission($groupId, $permissionCode) { ... }
  public function hasContentPermission($recordId, $permissionCode) { ... }
  public function hasModulePermission($moduleId, $recordId, $permissionCode) { ... }
  public function canLogIn() { ... }
  public function isLoggedIn() { ... }
  public function setCanLogIn($canLogIn) { ... }

}

Становится ли это "классом Бога"?

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

Полагаю, я мог бы поместить связанные с разрешениями методы в некоторый класс Permission, сделав эти методы статичными (например, :: userIsGroupAdministrator (...), :: userIsMemberOfGroup (...) :: userHasGroupPermission (...) , :: userHasContentPermission (...))

Любые предложения о том, как этот класс может быть лучше?

Answer 1

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

class User {
  String username
  String name
  String emailAddress
  Boolean active
  Integer permission # A bitmask of the statics in the Permission class
}

class Group {
  String name
}

class UserGroupMapping {
  User user
  Group group
  Boolean administrator
}

class Permission {
  static IS_ROOT = 1
  static IS_SITE_ADMIN = 2
}

class Session {
  User user
  Boolean logged_in
}

Остальное действительно относится к классу обслуживания:

<pre> class SecurityService { static public function hasModulePermission($user, $module, $record, $permission) { ... } static public function hasGroupPermission($user, $group, $permission) { ... } static public function hasContentPermission($user, $record, $permission) { ... } static public function hasModulePermission($user, $module, $record, $permission) { ... } }

Answer 2

Да, я думаю, что ваш класс User делает слишком много.

Разделение некоторых связанных с группой функций на класс Group и связанных с модулем функций на класс Module. Или, может быть, разделить разрешения на собственный набор классов.

Answer 3

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

Answer 4

Насколько сложно сейчас добавить новые функции или сохранить существующие в вашем приложении? Если у вас нет проблем в этом отношении, то вы, вероятно, в порядке, сохраняя все как есть (по крайней мере, на некоторое время).

Да, у вашего класса User есть некоторые пересекающиеся обязанности, которые потенциально могут быть преобразованы в систему контроля доступа на основе ролей .

Суть: вы действительно будете нуждаться в этом ?

Answer 5

Поможет ли вам описать, как ведет себя ваш объект User, с кем все это взаимодействует, какова его среда и т. Д.

Полагаю, я мог бы поместить методы, связанные с разрешениями, в некоторый класс Permission [...]

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

Я бы также посоветовал вам взять фиксированное состояние User, т. Е. Адрес электронной почты / имя и т. Д., И поместить его в отдельный класс.

Answer 6

Оставь эти ... они находят ИМО:

public function isLoggedIn() { ... }
public function getEmailAddress() { ... }
public function setEmailAddress() { ... }

Предлагаем заменить их функциями Get / Set Name, которые обрабатывают экземпляр TUserName:

public function getUsername() { ... }
public function setUsername($value) { ... }
public function getName() { ... }
public function setName($value) { ... }

TUserName будет иметь:

.FirstName
.LastName
.MiddleName
.UserName

Предлагаем заменить их функциями Get / Set Admin, которые обрабатывают экземпляр TUserGroup:

public function isGroupAdministrator($groupId) { ... }
public function isMemberOfGroup($groupId) { ... }
public function isSiteAdministrator() { ... }
public function isRoot() { ... }

TUserGroup будет иметь:

.MemberOfGroup(GroupID)
.AdminSite
.Root
.AdminGroup(GroupID)

Предложите заменить их функциями Get / Set Permission, которые обрабатывают экземпляр TUserPermissions:

public function hasModulePermission($moduleId, $recordId, $permissionCode) 
public function hasGroupPermission($groupId, $permissionCode) 
public function hasContentPermission($recordId, $permissionCode) 
public function hasModulePermission($moduleId, $recordId, $permissionCode) 
public function canLogIn() 
public function setCanLogIn($canLogIn)

TUserPermissions будет иметь:

.ModulePermitted(ModuleID,RecordID,PermCode)
.GroupPermitted(GroupID,PermCode)
.ContentPermitted(RecordID,PermCode)
.ModulePermitted(ModuleID,RecordID,PermCode)
.CanLogIn

Answer 7

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

В целом, я мог бы рекомендовать думать о вашем Пользователе как о центральном пункте для агрегации и составления (а не наследования) субъединиц функциональности - подхода, который достигает статуса модного слова в разработке игр под рубрикой «система сущностей».

Answer 8

Пока методы класса возвращают данные экземпляра или изменяют только внутреннее состояние, с классом все в порядке.

Но, например, Если метод canLogIn () ищет какую-либо внешнюю службу (репозиторий, служба членства и т. д.), возникает проблема простоты и тестируемости. Если это так, у вас должен быть какой-то класс обслуживания и эти методы. Как

new SomeFactory().GetUserService().canLogIn(user);

Answer 9

Это выглядит достаточно правдоподобно. Конечно, я не знаю приложения. Я думаю, что имеет смысл выделить класс Permissions.

В идеале, вы бы подумали о том, что значит иметь класс Permissions - что он знает, каковы его реальные действия? Кроме того, поскольку «полномочия» множественного числа, вы можете задаться вопросом, действительно ли вам нужна коллекция отдельных объектов «Разрешения», но действительно ли они являются доступом к простому логическому значению, которое может быть избыточным.

Answer 10

Вам, кажется, нужны отдельные объекты ACL и Role от объекта User.
http://en.wikipedia.org/wiki/Role-based_access_control

Вы также можете увидеть , как это делается в ZF .

Для удобства чтения вы можете использовать __get() и __set() вместо:

  public function setName($value) { ... }
  public function setEmailAddress($value) { ... }
  public function setUsername($value) { ... }
  public function getName() { ... }
  public function getEmailAddress() { ... }
  public function getUsername() { ... }