de-vraag
さらに
質問

特定の権限が他の権限を意味する場合のユーザー権限の設定

このクラスの目的は、ユーザーに権限を設定することです。これらには、削除、ダウンロード、アップロード、表示などが含まれます。これらのプロパティは他のプロパティに依存することが多く、それらを設定すると副作用があります。

たとえば、 CanUpload = false を設定した場合、アップロード権限がないと管理者になれないため、 CanManagePermissions = false も設定する必要があります。同様に、 CanManagePermissions = true を設定した場合は、削除、ダウンロード、およびアップロードをtrueに設定する必要があります。さらに、 CanView 権限を持たないと CanDownload 権限を持つことはできませんので、私も設定しました。しかし、 CanDownload なしで CanView を持つことができます。

あなたはこれが少し複雑になり始めるのを見ることができます。特に、関係を緩めたり厳しくしたり、いくつかの権限を強制的に無効にしたりするアカウント設定を通過している場合（これはUIにバインドしており、チェックボックスを有効または無効にしたいので重要です）。

The code works as-is, but it is very difficult to maintain and understand all of the relationships and side-effects. If someone is new to the code it can be a trip trying to figure out how properties depend on and change other properties (even more so when you get to doubly nested dependencies like CanManagePermissions->CanDownload->CanView).

関係をより明確にしたり、扱いやすくすることができますか？私はそれが単なる文書の問題以上のものであると思いますが、そうではないかもしれません。

セッター全員が最後に呼び出す1つの大きなメソッドを持つことが考えられました。その1つの方法で、すべての関係を強制することができます。これがすべての条件を満たすことができるかどうか、またはそれがそれでも良い考えであるかどうかわからない。

public class AccessControlViewModel : INotifyPropertyChanged
{
    private AccountPreferences Preferences => CurrentUser.Account.Preferences;

    private bool AllowDownloadNotificationsWithoutAdmin => Preferences.AllowDownloadNotificationsWithoutAdmin;

    private bool ShowDownloadLinkInUploadNotification => Preferences.ShowDownloadLinkInUploadNotification;

    public AccessControl AccessControl { get; private set; }

    public bool HasChanged { get; private set; }

    public bool CanUpload
    {
        get
        {
            return AccessControl.CanUpload.GetValueOrDefault();
        }
        set
        {
            if (AccessControl.CanUpload != value)
            {
                AccessControl.CanUpload = value;
                RaisePropertyChanged(nameof(CanUpload));

                if (!value)
                {
                    CanManagePermissions = false;
                    if (EnableViewOnly)
                    {
                        CanView = true;
                    }
                    else
                    {
                        CanDownload = true;
                    }
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(PermissionSummary));
            }
        }
    }

    public bool CanDownload
    {
        get
        {
            return AccessControl.CanDownload.GetValueOrDefault();
        }
        set
        {
            if (AccessControl.CanDownload != value)
            {
                AccessControl.CanDownload = value;
                RaisePropertyChanged(nameof(CanDownload));

                if (!value)
                {
                    CanManagePermissions = false;
                    if (!EnableViewOnly)
                    {
                        CanView = false;
                    }
                    if (!EnableNotifyOnUpload)
                    {
                        NotifyOnUpload = false;
                    }
                }
                else
                {
                    CanView = true;
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(EnableNotifyOnUpload));
                RaisePropertyChanged(nameof(PermissionSummary));
            }
        }
    }

    public bool CanDelete
    {
        get
        {
            return AccessControl.CanDelete.GetValueOrDefault();
        }
        set
        {
            if (AccessControl.CanDelete != value)
            {
                AccessControl.CanDelete = value;
                RaisePropertyChanged(nameof(CanDelete));

                if (!value)
                {
                    CanManagePermissions = false;
                }
                else
                {
                    CanDownload = true;
                    CanUpload = true;
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(PermissionSummary));
            }
        }
    }

    public bool EnableViewOnly => Preferences.EnableViewOnly;

    public bool CanView
    {
        get
        {
            return AccessControl.CanView.GetValueOrDefault();
        }
        set
        {
            if (AccessControl.CanView != value)
            {
                AccessControl.CanView = value;

                if (!value)
                {
                    CanDownload = false;
                    CanUpload = true;
                }
                if (!EnableNotifyOnUpload)
                {
                    NotifyOnUpload = false;
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(CanView));
                RaisePropertyChanged(nameof(EnableNotifyOnUpload));
                RaisePropertyChanged(nameof(PermissionSummary));
            }
        }
    }

    public bool CanManagePermissions
    {
        get
        {
            return AccessControl.CanManagePermissions.GetValueOrDefault();
        }
        set
        {
            if (AccessControl.CanManagePermissions != value)
            {
                AccessControl.CanManagePermissions = value;
                RaisePropertyChanged(nameof(CanManagePermissions));

                if (value)
                {
                    CanDelete = true;
                    CanUpload = true;
                    CanDownload = true;
                }
                else if (!EnableNotifyOnDownload)
                {
                    NotifyOnDownload = false;
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(EnableNotifyOnDownload));
                RaisePropertyChanged(nameof(PermissionSummary));
            }
        }
    }

    public bool EnableNotifyOnUpload => CanDownload || (CanView && !ShowDownloadLinkInUploadNotification);

    public bool NotifyOnUpload
    {
        get
        {
            return AccessControl.NotifyOnUpload.GetValueOrDefault();
        }
        set
        {
            if (NotifyOnUpload != value && CanDownload)
            {
                AccessControl.NotifyOnUpload = value;
                HasChanged = true;
            }
            else
            {
                AccessControl.NotifyOnUpload = false;
            }

            RaisePropertyChanged(nameof(NotifyOnUpload));
        }
    }

    public bool EnableNotifyOnDownload => CanManagePermissions || AllowDownloadNotificationsWithoutAdmin;

    public bool NotifyOnDownload
    {
        get
        {
            return AccessControl.NotifyOnDownload.GetValueOrDefault();
        }
        set
        {
            if (NotifyOnDownload != value)
            {
                AccessControl.NotifyOnDownload = value;

                if (value && !AllowDownloadNotificationsWithoutAdmin)
                {
                    CanManagePermissions = true;
                }

                HasChanged = true;
                RaisePropertyChanged(nameof(NotifyOnDownload));
            }
        }
    }

    public string PermissionSummary
    {
        get
        {
            //...Returns a string based on the above properties...
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    private void RaisePropertyChanged([CallerMemberName]string propertyName = "")
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}
コードレビュー
c#
properties
wpf

ビジネスロジック（「アップロード許可なしで管理者になれないので、 CanUpload = false を設定し、次に CanManagePermissions = false を設定する必要があります」）が混在しているようです。ユーザーインターフェイスのプレゼンテーションに追いついた。

AccessControlViewModel は、ユーザーがフォーム上で選択したものを表す厳密に型指定されたデータの袋です。ボタンを押した後、フォーム内のエントリを検証する必要があります。

検証に合格した後は、「問題ドメイン」を表すクラスのコレクション、つまり User クラスのように権限を管理する必要があります。

public class User
{
    public bool CanUpload { get; private set; }
    public bool CanManagePermissions { get; private set; }

    public void CanAllowUploads
    {
        get { return CanManagePermissions; }
    }

    public void AllowUploads()
    {
        if (!CanAllowUploads)
            throw new InvalidOperationException("Cannot allow uploads without being able to manage permissions");

        CanUpload = true;
    }

    public void DisallowUploads()
    {
        CanUpload = false;
    }

    public void AllowPermissionManagement()
    {
        CanManagePermissions = true;
    }

    public void DisallowPermissionManagement()
    {
        CanUpload = false;
        CanManagePermissions = false;
    }
}

ここではアップロードと管理権限の関係が強化されています。

User user = new User();

user.AllowUploads();//Throws InvalidOperationException

user.AllowPermissionManagement();
user.AllowUploads();//No exception gets thrown

user.CanUpload           //Is True
user.CanManagePermissions//Is True

user.DisallowPermissionManagement();

user.CanUpload           //Is False
user.CanManagePermissions//Is False

ビジネスロジックとは別のUIロジックが本当に必要です。

すべての権限をチェックし、現在の状態が無効な場合は修正する必要がある単一の ValidatePermissions メソッドにすべてのロジックを移動することを強くお勧めします。こうすれば、すべての依存関係を理解し​​て維持するのがずっと簡単になります。 正しいパーミッション設定がアプリケーションにとって重要な場合は、検証ロジックをビジネスレイヤに移動することも検討する必要があります。多くの場合、これらの設定はどこかに保存され（DB、 .ini ファイル、クラウドなど）、アプリケーションを中断したくはありません。理由はなんらかの理由で変更されたからです。あなたの申請。

また、 CallerMemberName 属性を使用している場合は、 nameof（）演算子を使用する必要はありません。 RaisePropertyChanged（）を呼び出すことができます（引数なしで）。

AccessControl property is fishy. If it is actually a wpf control, than this is a major violation of MVVM. Your viewmodels should never have a direct access to view, thats what databinding is for. If it is not a UI control, then IMHO you should rename it, removing the Control word from both class name and property name. AccessManager sounds good to me.

Instead of using bool properties, use bit flags with enumeration types. Just make sure that the values assigned to the different [permission] flags are in increasing powers of 2 (e.g. 0, 1, 2, 4, 8, 16, etc.)

enum Permissions { None = 0, CanView = 1, CanDownload = 2, CanManagePermissions = 4 }

class User { public Permissions Permissions { get; set; } }

...
user.Permissions = Permissions.CanView | Permissions.CanDownload;

Probably not the Modern (tm) approach, but I would set this these up with two layers: base permissions and effective or resolved permissions. You could implement this by having the get{} for a property check the dependencies of the current property with an AND (&&) to each property that it depends on.

これは私が行っているものです。すべてのプロパティをバッキングフィールドで単純に取得/設定するように変更してから、 PropertyChanged を監視して副作用を発生させます。私はそれがもっと読みやすいと思います（私はそれに満足しているので十分です）。

private void AccessControlViewModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
{
    var setHasChanged = false;
    switch (e.PropertyName)
    {
        case nameof(CanView):
            {
                if (!CanView)
                {
                    CanDownload = false;
                    CanUpload = true;
                }
                setHasChanged = true;
                RaisePropertyChanged(nameof(EnableNotifyOnUpload));
                break;
            }
        case nameof(CanDownload):
            {
                if (CanDownload)
                {
                    CanView = true;
                }
                else
                {
                    NotifyOnUpload = false;
                    CanManagePermissions = false;
                    if (!EnableViewOnly || ShowDownloadLinkInUploadNotification)
                    {
                        CanUpload = true;
                    }
                    else if (!CanUpload)
                    {
                        CanDelete = false;
                    }
                }
                setHasChanged = true;
                RaisePropertyChanged(nameof(EnableNotifyOnUpload));
                break;
            }
        case nameof(CanUpload):
            {
                if (!CanUpload)
                {
                    if (!EnableViewOnly)
                    {
                        CanDownload = true;
                    }
                    if (!CanDownload)
                    {
                        CanDelete = false;
                    }
                    CanView = true;
                    CanManagePermissions = false;
                }
                setHasChanged = true;
                break;
            }
        case nameof(CanDelete):
            {
                if (CanDelete)
                {
                    CanView = true;
                    CanDownload = true;
                }
                else
                {
                    CanManagePermissions = false;
                }
                setHasChanged = true;
                break;
            }
        case nameof(CanManagePermissions):
            {
                if (CanManagePermissions)
                {
                    CanView = true;
                    CanDownload = true;
                    CanUpload = true;
                    CanDelete = true;
                }
                setHasChanged = true;
                RaisePropertyChanged(nameof(EnableNotifyOnDownload));
                break;
            }
        case nameof(NotifyOnDownload):
        case nameof(NotifyOnUpload):
            {
                setHasChanged = true;
                break;
            }
    }
    if (setHasChanged)
    {
        HasChanged = true;
        if (!EnableNotifyOnDownload)
        {
            NotifyOnDownload = false;
        }
        if (!EnableNotifyOnUpload)
        {
            NotifyOnUpload = false;
        }
        RaisePropertyChanged(nameof(PermissionSummary));
    }
}