Se una classe utente contiene solo attributi e nessun metodo oltre a getter / setter?

4

Sto cercando di migliorare il mio codice OOP e penso che la mia classe User stia diventando troppo grassa.

Nel mio programma un utente ha i diritti su "liste". Leggi, scrivi, aggiorna, cancella. Così ho creato una classe utente

class User
{
protected $_id;
protected $_email;
protected $_username;
protected $_hashedPassword;

//...Various setters/getters

public function canRead(List $list){
    //Database query verifies if user has READ rights
}

public function canUpdate(List $list){
    //Database query verifies if user has UPDATE rights
}
//etc...
}
  • I metodi canRead, canUpdate, canWrite, canDelete possono essere spostati in un'altra classe (UserAccessCheck o qualcosa del genere ...)?
  • In caso contrario, l'attuale SQL deve essere spostato nell'oggetto List (listCanBeReadByUser ())?
posta Lukmo 10.04.2013 - 13:45
fonte

6 risposte

5

Quello che stai facendo è mettere insieme un meccanismo per il controllo degli accessi (liste) o ACL. In generale, come dice Ritesh , è una cattiva idea eseguirlo tutto in una classe.

Questa risposta su SO ha un modo migliore di gestire il controllo degli accessi.

Invece di estendere la tua classe User , devi avvolgere la tua classe User in SecureContainer . Vedi la risposta collegata per i dettagli, ma la chiamata generale sarebbe:

// assuming that you have two objects already: $currentUser and $controller
$acl = new AccessControlList( $currentUser );

$controller = new SecureContainer( $controller, $acl );
// you can execute all the methods you had in previous controller 
// only now they will be checked against ACL
$controller->actionIndex();

Ciò consente alla classe User di concentrarsi su ciò che deve fare e consente alla classe AccessControlList di concentrarsi su ciò che esso deve fare.

    
risposta data 10.04.2013 - 14:27
fonte
2

Una classe dovrebbe avere un solo scopo di esistere.

I metodi canRead, canUpdate, canWrite, canDelete possono essere spostati in un'altra classe (UserAccessCheck o qualcosa del genere ...)?

Sì, questi metodi dovrebbero essere messi in classi separate.

    
risposta data 10.04.2013 - 14:05
fonte
2
  • se l'autorizzazione è in qualche modo una proprietà dell'utente, è il posto naturale in cui memorizzare i dati
  • se il permesso è una proprietà dell'elenco, questo è il luogo naturale in cui archiviarlo
  • se il test per il permesso è naturalmente un azione sull'utente (cioè, quella sintassi si adatta meglio al tuo schema di utilizzo), questo è il posto naturale in cui inserire il metodo
  • se l'elenco non dovrebbe conoscere l'utente e l'utente non dovrebbe conoscere l'elenco, ma alcuni ACL separati dovrebbero conoscere sia le che loro relazioni, forse che è dove devono andare i dati e / o il metodo
  • ecc. ecc.

In generale, sembrano tre preoccupazioni distinte (o separabili): descrivere un utente, archiviare un elenco e gestire l'accesso di uno all'altro.

Se scegli di combinare due di questi problemi nella stessa classe (e se sì, quali), dovrebbe essere perché questa disposizione emerge naturalmente dal tuo modello ( ... è una proprietà di ... ), o perché è più conveniente o efficiente. Non ci hai detto abbastanza per fare quella scelta per te.

    
risposta data 10.04.2013 - 14:29
fonte
1

Mi piace l'idea di mettere la logica aziendale in classi modello come Utente. In effetti, l'ho fatto esattamente prima con metodi privilegiati come canDoBlah ().

Rende il codice di facile lettura quando si utilizza l'oggetto nel livello dell'interfaccia utente:

public void processList(User user, List list) {
    if (user.canReadList(list)) {
        // do some work
    }
}

Suppongo che potresti inserire la logica nella classe List come hai suggerito con listCanBeReadByUser (), che a tutti gli effetti è uguale, ma penso che sia più facile leggere sull'oggetto User.

L'unico suggerimento che vorrei fare è evitare di inserire SQL nella classe del modello. Questo appartiene al livello di persistenza, forse in un DAO .

    
risposta data 10.04.2013 - 14:02
fonte
-1

L'astrazione è l'idea.

Mappare tutte le azioni che si desidera raggiungere nella classe utente

Qualsiasi azione non pertinente a quell'oggetto dovrebbe essere rimossa.

    
risposta data 10.04.2013 - 15:30
fonte
-1

L'astrazione è uno dei principi fondamentali di OOP

la corretta mappatura degli oggetti fisici a quelli astratti che sono programmati è la chiave.

Aiuta a determinare come vuoi rappresentare questi oggetti.

    
risposta data 12.04.2013 - 02:08
fonte

Leggi altre domande sui tag