qual è il modo migliore per scrivere un codice ricorrente con enumerazioni diverse

if a new role is added I'd have to write an if condition for it which doesn't feel right

Se viene aggiunto un nuovo ruolo, chiamiamolo NewRole , quindi dovrai anche creare un enum di NewRolePages . Ciò significa sempre che devi scrivere qualcosa di nuovo. Che si tratti di una condizione if, una nuova voce in una tabella di mappatura, un nuovo gestore, ... lo schema utilizzato può cambiare ma il fatto che sia necessario aggiungere qualcosa non lo è.

Tuttavia, penso che ci sia un problema più centrale qui. Per una determinata pagina, è ragionevole che più ruoli possano accedervi, e quindi finirai con la stessa pagina menzionata più volte: una volta in ogni FooPages per ogni Foo di ruolo che dovrebbe avere accesso.

Questo viola la DRY, dato che ora sei bloccato con la necessità di sincronizzare più enumerazioni qualora un nome di pagina dovesse cambiare. Stai abusando efficacemente delle tue enumerazioni come una lista di stringhe. Presumibilmente, lo stai facendo perché vuoi Intellisense, ma non è proprio questo il modo per farlo.

Un approccio molto più riusabile sarebbe quello di ridefinire i tuoi ruoli come Flags enum e crea un mapping tra il nome della pagina e l'enum combinato.

[Flags]
public enum Roles
{
    Admin = 1,
    CompanyOwner = 2, 
    Customer = 4, 
    Guest = 8, 

    Everyone = 15,  // is the same as combining the above 4 roles, since 15 = 8+4+2+1
}

Questo significa che puoi combinare i valori:

Role rolesForSettingsPage = Role.Admin | Role.CompanyOwner;

Tuttavia, si desidera mappare queste enumerazioni (combinate) a un valore stringa. Per questo, puoi impostare un dizionario:

private Dictionary<string, Role> _pageRoles = new Dictionary<string, Role>()
{
    { "Welcome", Role.Everyone },
    { "Settings", Role.Admin | Role.CompanyOwner },
    { "CustomerInfo", Role.Admin | Role.Customer },
    { "GuestPage", Role.Guest }
}

E poi puoi usare il dizionario per controllare se il ruolo dato ha accesso alla pagina data

public bool RoleCanAccessPage(Role userRole, string url)
{
    string pageName = ExtractPageNameFromUrl(url);

    return _pageRoles.ContainsKey(pageName)  //Does this page have a mapping?
           &&
           (_pageRoles[pageName] & userRole) != 0;  //Does the user have at least one of the mapped roles
}

Questo ti dà anche la possibilità per un utente di avere più ruoli, ma è facoltativo e può essere ignorato se non ne hai bisogno.

Inoltre, nota che in questo esempio non hai dati copiati / incollati come il nome della pagina.

Nota
La mia risposta rimane il più vicino possibile alla tua mappatura string-to-enum. Ci sono altri modi per farlo, ma richiedono cambiamenti architettonici più grandi e richiederebbero più tempo e sforzi per mostrare / spiegare / implementare. La mia risposta è semplicemente una delle tante possibili risposte.

Nota 2
Se lo volessi davvero, potresti comunque creare un enum Page che contenga tutte le pagine (non solo separate per ruolo!), E quindi il tuo dizionario può essere modificato in Dictionary<Page, Roles>() .

Commenti e miglioramenti minori

Questi commenti non sono applicabili al mio miglioramento suggerito ma sono comunque buoni consigli

• Per le buone pratiche, le enumerazioni dovrebbero avere un nome univoco (ad esempio Page ). L'unica eccezione qui è enumerazione di bandiere, dove è preferito il plurale (ad esempio [Flag] Ruoli ').
• Come regola generale, evitare il cast non necessario dei valori string / int quando si ha a che fare con queste enumerazioni. Non utilizzare int roleID . Le enumerazioni esistono per un motivo, quindi usale: Role userRole . Ciò significa che non è necessario eseguire il cast e che il codice che chiama il metodo sarà molto più leggibile.
• Utilizza un switch invece di istruzioni if separate.
• Puoi utilizzare .Any() di LINQ anziché foreach . Per il tuo codice, le predizioni potrebbero essere modificate in Enum.GetNames(typeof(AdminPages)).Any(name => name == cleanedUrl)
• AllowPageAccess non è un buon nome di metodo. Stai restituendo un valore booleano, quindi il tuo metodo dovrebbe porre una domanda. AllowPageAccess sarebbe il nome di un metodo con fa (imposta) qualcosa. IsPageAccessAllowed sarebbe il nome di un metodo che restituisce un valore booleano che indica se è consentito l'accesso alla pagina.

Se hai la possibilità di cambiare l'architettura da Enums, puoi aggiungere una classe per gestire la tua logica. Ecco un esempio (molto semplice) di come potrebbe essere questo.

public class MyPage
{
    public string Url { get; }
    public int AccessLevel { get; }

    public MyPage(string url, int accesslevel)
    {
        Url = url;
        AccessLevel = accesslevel;
    }

    public bool UserCanAccess(int roleId)
    {
        return roleId >= AccessLevel;
    }
}

Supponiamo ora di avere un elenco di pagine configurate in questo modo:

List<MyPage> pages = new List<MyPage>()
{
    new MyPage("test1", 1), // Allow guests and up
    new MyPage("test2", 2), // Allow Customers and up
    new MyPage("test3", 3), // Allow CompanyOwners and up
    new MyPage("test4", 4), // Allow Admins
    new MyPage("test5", 2)
};

Il tuo metodo AllowPageAccess sarà simile a questo:

return pages.FirstOrDefault(p => p.Url == url).UserCanAccess(roleId);

Hai un enum con una serie di valori tipo e un numero di classi, ognuna delle quali corrisponde esattamente a un valore enum . Ma l'associazione tra la classe concreta e il valore enum non è esplicita, quindi devi ripetere un intero blocco di codice quando vuoi effettivamente usare questa associazione.

Ciò che dovresti fare è mantenere una mappa che mappa i valori di enum nelle classi del gestore o (ancora meglio) fare riferimento alla classe del gestore appropriato direttamente nella definizione dei tuoi valori di enum . In entrambi i casi, puoi scrivere il codice foreach ... typeof solo una volta.