runceel / ReactiveProperty

ReactiveProperty provides MVVM and asynchronous support features under Reactive Extensions. Target frameworks are .NET 6+, .NET Framework 4.7.2 and .NET Standard 2.0.
MIT License
896 stars 100 forks source link

Is there a simple way to validate and clamp a value? #284

Closed josephnarai closed 3 years ago

josephnarai commented 3 years ago

Hi,

All the example for DataValidation seem to be about validating a string and displaying an error, so I'm not sure if there is a simple way to just limit the range of an integer value?

So I have ReactiveProperty Width;

And I want to limit it from 5 to 1000.

So I just want to do Width.Value = Math.Max(Math.Min(value, 1000), 5);

I can of couse make an extra property setter that does this validation, but it would be nice to be integrated into the property so this check gets done before the Value change is propogated.

Thank you.

josephnarai commented 3 years ago

I've created this which works nicely:

public class ClampedReactivePropertySlim<T> : ReactivePropertySlim<T> where T : IComparable<T>
{
    public T MinValue { get; private set; }
    public T MaxValue { get; private set; }

    public ClampedReactivePropertySlim(T initialValue, T minValue, T maxValue)
    {
        MinValue = minValue;
        MaxValue = maxValue;
        Value = initialValue;
    }

    public new T Value
    {
        get
        {
            return base.Value;
        }
        set
        {
            base.Value = Clamp(value);
        }
    }

    private T Clamp(T value)
    {
        if (value.CompareTo(MinValue) < 0)
        {
            value = MinValue;
        }
        if (value.CompareTo(MaxValue) > 0)
        {
            value = MaxValue;
        }
        return value;
    }
}

Please let me know if there is a better way to implement it.

Thank you.

soi013 commented 3 years ago

How about RangeAttribute ?

public class MainWindowViewModel
{
    [Range(5, 1000)]
    public ReactiveProperty<int> TargetWithRange { get; }
    public MainWindowViewModel()
    {
        TargetWithRange = new ReactiveProperty<int>()
            .SetValidateAttribute(() => TargetWithRange);
    }
}

https://docs.microsoft.com/dotnet/api/system.componentmodel.dataannotations.rangeattribute

Does it fit your requirements?

runceel commented 3 years ago

@josephnarai Thank you for using ReactiveProperty. :)

@soi013 Thank you for answering for this issue. It is the correct way to validate input values.

@josephnarai If you want to force the range of values to be from 5 to 1000, you can define and use the following extension method.

using Reactive.Bindings;
using System.ComponentModel.DataAnnotations;

var vm = new VM();
vm.Width.Value = 9999; // invalid
Console.WriteLine(vm.Width.Value); // 1000
vm.Width.Value = 100; // valid
Console.WriteLine(vm.Width.Value); // 100
vm.Width.Value = -100; // invalid
Console.WriteLine(vm.Width.Value); // 5

class VM
{
    [Range(5, 1000)]
    public ReactiveProperty<int> Width { get; }

    public VM()
    {
        Width = new ReactiveProperty<int>(5)
            .Comp(5, 1000);
    }
}

public static class ReactivePropertyExtensions
{
    public static TReactiveProperty Comp<TReactiveProperty, T>(this TReactiveProperty self, T minValue, T maxValue)
        where TReactiveProperty : IReactiveProperty<T>
    {
        self.Subscribe(x =>
        {
            if (Comparer<T>.Default.Compare(x, minValue) < 0) { self.Value  = minValue; }
            if (Comparer<T>.Default.Compare(x, maxValue) > 0) {  self.Value = maxValue; }
        });
        return self;
    }
}

However, the behavior is that ReactiveProperty store an invalid value and then it is changed to a valid value. If you want to force value in range minValue to maxValue, then you have to implement it yourself you did.

The following code snippet is my implementation if I want the feature.

using Reactive.Bindings;
using System.Collections;
using System.ComponentModel;

var vm = new VM();
vm.Width.Value = 9999; // invalid
Console.WriteLine(vm.Width.Value); // 1000
vm.Width.Value = 100; // valid
Console.WriteLine(vm.Width.Value); // 100
vm.Width.Value = -100; // invalid
Console.WriteLine(vm.Width.Value); // 5

class VM
{
    public IReactiveProperty<int> Width { get; }

    public VM()
    {
        Width = new ReactiveProperty<int>(5)
            .Comp(5, 1000);
    }
}

static class IReactivePropertyExtensions
{
    // An extension method to wrap ReactiveProperty using CompReactiveProperty.
    public static IReactiveProperty<T> Comp<T>(this IReactiveProperty<T> self, T minValue, T maxValue) =>
        new CompReactiveProperty<T>(self, minValue, maxValue);
}

// CompReactiveProperty
// This is a just a wrapper of IReactiveProperty<T> to force value range to be minValue to maxValue
class CompReactiveProperty<T> : IReactiveProperty<T>
{
    private readonly IReactiveProperty<T> _source;
    private readonly T _minValue;
    private readonly T _maxValue;

    public T Value { get => _source.Value; set => _source.Value = Comp(value); }

    public CompReactiveProperty(IReactiveProperty<T> source, T minValue, T maxValue)
    {
        _source = source;
        _minValue = minValue;
        _maxValue = maxValue;
        _source.Value = Comp(Value);
    }

    private T Comp(T value)
    {
        if (Comparer<T>.Default.Compare(value, _minValue) < 0) { return _minValue; }
        if (Comparer<T>.Default.Compare(value, _maxValue) > 0) { return _maxValue; }
        return value;
    }

    public IObservable<IEnumerable> ObserveErrorChanged => _source.ObserveErrorChanged;

    public IObservable<bool> ObserveHasErrors => _source.ObserveHasErrors;

    public bool HasErrors => _source.HasErrors;

    object IReactiveProperty.Value { get => Value; set => Value = (T)value; }

    T IReadOnlyReactiveProperty<T>.Value => Value;

    object IReadOnlyReactiveProperty.Value => Value;

    public event PropertyChangedEventHandler? PropertyChanged
    {
        add { _source.PropertyChanged += value; }
        remove { _source.PropertyChanged -= value; }
    }
    public event EventHandler<DataErrorsChangedEventArgs>? ErrorsChanged
    {
        add { _source.ErrorsChanged += value; }
        remove { _source.ErrorsChanged -= value; }
    }

    public void Dispose() => _source.Dispose();

    public void ForceNotify() => _source.ForceNotify();

    public IEnumerable GetErrors(string? propertyName) => _source.GetErrors(propertyName);

    public IDisposable Subscribe(IObserver<T> observer) => _source.Subscribe(observer);
}

And if you can separate two properties for input and output, then you can use the following approach:

using Reactive.Bindings;
using System.Reactive.Linq;

var vm = new VM();
vm.Width.Value = 9999; // invalid
Console.WriteLine(vm.ValidWidth.Value); // 1000
vm.Width.Value = 100; // valid
Console.WriteLine(vm.ValidWidth.Value); // 100
vm.Width.Value = -100; // invalid
Console.WriteLine(vm.ValidWidth.Value); // 5

class VM
{
    // For input
    public ReactiveProperty<int> Width { get; }
    // For output
    public ReadOnlyReactivePropertySlim<int> ValidWidth { get; }

    public VM()
    {
        Width = new ReactiveProperty<int>();
        ValidWidth = Width
            .Select(x => x switch
            {
                < 5 => 5,
                > 1000 => 1000,
                _ => x,
            })
            .ToReadOnlyReactivePropertySlim();
    }
}

I hope one of these will be of some help to you.

josephnarai commented 3 years ago

Thank you - I think my solution is the simplest :)

I apprecaited your great work on ReactiveProperties!

runceel commented 3 years ago

@josephnarai I think your implementation is not work some situation.

For example:

ReactiveePropertySlim<int> x = new ClampedReactivePropertySlim<int>(5, 5, 1000);
x.Value = 9999;
Console.WriteLine(x.Value); // I think output of this line is 9999.

The same issue will be occurred the following cases:

IReactiveProperty x = new ClampedReactivePropertySlim<int>(5, 5, 1000);
IReactiveProperty<int> y = ClampedReactivePropertySlim<int>(5, 5, 1000);
josephnarai commented 3 years ago

Yes, that is correct, your example will not work properly as you have declared a ReactivePropertySlim x

Correct usage for my class is:

ClampedReactivePropertySlim x = new ClampedReactivePropertySlim(5, 5, 1000); x.Value = 9999; Console.WriteLine(x.Value); // output of this line is 1000

Same with your second example, you must use it as ClampedReactivePropertySlim. I did not create an extension to your class, but a new class that overrides the Value property, which I think it a much more efficient way of doing the Clamp or Range limiting of values.

runceel commented 3 years ago

If you have a plan to use EventToReactiveProperty or EventToReactiveCommand with ClampedReactivePropertySlim on your app, it doesn't work correctly because EventToReactiveXxxxxx classes are handling an instance of ReactiveProperty as IReactiveProperty interface.

If you don't have the plan, then no problem.

Have a happy coding! And thank you for using this library. 😀

josephnarai commented 3 years ago

That makes sense. I'm not using the Clamped class with the conversions or Interfaces.

Thanks!