push-pop / Unity-MVVM

Lightweight MVVM Framework for Unity3D
MIT License
323 stars 27 forks source link

Strange order of types in RemapValueConverter #64

Closed AAAYaKo closed 3 years ago

AAAYaKo commented 3 years ago

now

            if (value is Single)
            {
                var retVal = ((Single)value).Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);

                return _floorToInt ? System.Convert.ChangeType(Mathf.FloorToInt(retVal), targetType) : retVal;
            }
            else if (value is Single?)
            {
                var retVal = (value as Single?).Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);

                return retVal == null ? null : _floorToInt ? System.Convert.ChangeType(Mathf.FloorToInt(retVal.Value), targetType) : retVal;
            }

            else if (value is Double)
            {
                var retVal = ((Double)value).Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);

                return _floorToInt ? System.Convert.ChangeType(Mathf.Floor((float)retVal), targetType) : retVal;
            }

            else if (value is Double?)
            {
                var retVal = (value as Double?).Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);

                return retVal == null ? null : _floorToInt ? System.Convert.ChangeType(Mathf.FloorToInt((float)retVal.Value), targetType) : retVal;
            }

            else
                throw new NotImplementedException();

And that is strange Firstly nullable must be first

        static void Main(string[] args)
        {
            float? value = 1;

            if (value is Single)
            {
                Console.WriteLine("float");
            }
            else if (value is Single?)
            {
                Console.WriteLine("nullable float");
            }
           //Console: float

            if (value is Single?)
            {
                Console.WriteLine("nullable float");
            }
            else if (value is Single)
            {
                Console.WriteLine("float");
            }
            //Console: nullable float
        }

Secondly why not used pattern matching? I mean, I don't see some big difference between nullable and not nullable values, (because now it works) so i think it might be rewritten Something like this

        public override object Convert(object value, Type targetType, object parameter)
        {
            switch (value)
            {
                case float number:
                    number = number.Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);
                    return _floorToInt ? number.ToInt(targetType) : number;

                case double number:
                    number = number.Map(InputRange.x, InputRange.y, InputRange.x, OutputRange.y);
                    return _floorToInt ? number.ToInt(targetType) : number;

                case null:
                    return null;

                default:
                    Debug.LogWarning(
                        $"The property {value} is of an unsupported types. " +
                        $"Please use the float double or their nullable types");
                    return null;
            }
        }

And maybe rewrite Map() from this Map(this float value, float from1, float to1, float from2, float to2) to this Map(this float value, Vector2 from, Vector2 to) for clean up the code

AAAYaKo commented 3 years ago

And is it right that we map from input.x and input.y to input.x and output.y? I mean now we can't remap lower value 2021-02-25

push-pop commented 3 years ago

Fixed by https://github.com/push-pop/Unity-MVVM/pull/75/commits/b8456495eb99450891bde3615feea3d0281f3a84