2009-04-08 14 views
7

tengo la clase siguiente: "? Clase Dios"¿Qué es un mejor diseño?

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) { ... } 

} 

Es esto convertirse en una

No estoy seguro de si debería dividir esta clase. El problema al hacerlo es que los métodos de esta clase son utilizados por su dominio para determinar si mostrar determinados elementos de la IU en las páginas web, por lo que no hay ningún comportamiento en la clase.

Supongo que podría poner los métodos relacionados con permisos en alguna clase de permiso, haciendo que esos métodos sean estáticos (por ej. :: userIsGroupAdministrator (...), :: userIsMemberOfGroup (...) :: userHasGroupPermission (...), :: userHasContentPermission (...))

¿Alguna sugerencia sobre cómo esta clase podría ser mejor?

+0

I Sugiere etiquetar como 'php' o 'language-agnostic' para mejorar la visibilidad. –

Respuesta

8

Sí, creo que su clase de usuario está haciendo demasiado.

Divida algunas de las funciones relacionadas con el grupo en una clase de grupo, y las funciones relacionadas con el módulo en una clase de módulo. O tal vez dividir permisos en su propio conjunto de clases.

+0

¿Qué métodos crearías en estas clases? –

+0

El grupo puede tener getMembers y getAdministrators. Cosas similares van para los módulos. Sin embargo, es una buena idea centralizar el control de permisos. Alguna función central de havePermission() funcionaría. – Kalium

4

Es difícil de decir solo por los nombres de los métodos. Una heurística que utilizo para dividir una clase es observar las variables de estado. ¿Hay piezas de estado que tienden a usarse juntas? ¿Se pueden dividir en una clase separada?

0

Consideraría crear una clase de permisos y hacer que un miembro de la clase de usuario.

+0

Delegación e información oculta. Me gusta. –

0

Yo dividiría el aspecto de "usuario" y el aspecto de gestión de "grupo" de la clase.

0

Al menos eliminaría la función setCanLogIn($canLogIn). Esto realmente debe determinarse internamente dentro de la clase en función de si el usuario ha proporcionado las credenciales correctas y ha sido autenticado.

Dependiendo de qué tan grande sea este proyecto, o si esto es parte de un marco determinará si se necesita más abstracción.

+0

La razón por la que tengo este método es porque almaceno una bandera booleana para saber si el usuario puede iniciar sesión. –

0

Parece que necesita objetos separados ACL y Rol del objeto Usuario.
http://en.wikipedia.org/wiki/Role-based_access_control

También puede ver how it's done in ZF.

Para facilitar la lectura es posible considerar el uso de __get() and __set() en lugar de:

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

Definitivamente en la agenda. –

1

Esto ayudará si usted podría describir cómo se comporta el objeto User, que todos los que interactúa, lo que es medio ambiente, etc.

supongo que podría poner los métodos relacionados con los permisos de alguna clase de permiso [...]

Sí, puede consultar el diseño basado en políticas.

También le sugiero que elimine el estado fijo de User, es decir, correo electrónico/nombre, etc. y lo coloque en una clase separada.

0

Parece razonablemente plausible.No sé la aplicación, por supuesto. Creo que podría tener sentido factorizar una clase de Permisos.

Idealmente, entonces pensaría en lo que significa tener una clase de permisos: ¿qué sabe, cuáles son sus acciones reales? Además, dado que los "permisos" son plurales, puede preguntarse si realmente desea una colección de objetos de Permiso individuales, pero si esos son efectivamente accesos a un booleano simple que podría ser excesivo.

0

Siempre que los métodos en la clase devuelvan datos de instancia o solo modifiquen el estado interno, no hay nada de malo en la clase.

Pero, por ejemplo, si el método canLogIn() busca algún servicio externo (repositorio, servicio de membresía, etc.), existe un problema de simplicidad y capacidad de prueba. Si este es el caso, debe tener alguna clase de servicio y tener esos métodos en ella. Como

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

Si ya tiene el código en ejecución, entonces refactorícese en pedazos pequeños. Si no me gustaría ver lo siguiente:


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 
} 

El resto de esta realidad pertenece a una clase de servicio:

 
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) { ... } 
} 

+0

No hay forma de que pueda probar estos métodos estáticos. Este es sin duda un mal diseño desde el punto de vista de la capacidad de prueba. –

+0

No hay nada que probar excepto en la clase SecurityService. Todo lo demás es un envoltorio alrededor de una pieza de datos atómica. –

1

¿es muy difícil ahora para añadir nuevas funcionalidades o mantener la funcionalidad existente en tu aplicación? Si no tienes problemas en ese sentido, entonces probablemente estés bien manteniendo las cosas como están (al menos por un tiempo).

Sí, su clase de usuario tiene algunas responsabilidades que se superponen que podrían refactorizarse en un sistema Role-based access control.

La conclusión es: ¿realmente eres gonna need it?

0

No está tan mal, pero creo que la mecánica de tus permisos podría usar el factoring. Ese es un caso que claramente requiere agregación, no herencia, también. Si existe la posibilidad de que se asocie información de contacto no trivial con el usuario, es posible que también desee factorizar la dirección de correo electrónico, tal vez en una identidad que pueda tener otra información de contacto cargada si es necesario.

En general, podría recomendar pensar en su Usuario como un punto central para agregar y componer (no heredar) subunidades de funcionalidad, un enfoque que está logrando el estado de palabra de moda en el desarrollo de juegos bajo la rúbrica de 'sistema de entidad'.

0

Deje estos ...son encontrar IMO:

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

sugerimos que sustituirlos por get/set funciones Nombre, que manejan una instancia de TUserName:

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

TUserName tendría:

.FirstName 
.LastName 
.MiddleName 
.UserName 

sugieres reemplácelos con las funciones Administrar/Establecer, que manejan una instancia de TUserGroup:

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

TUserGroup tendría:

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

sugerimos que sustituirlos/Set funciones tienen permiso, que manejan una instancia de 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 tendría:

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

No estoy familiarizado con la "T" o "." notación. Qué significa eso? – rick

Cuestiones relacionadas