ronantakizawa/codereview-bench huggingface dataset:
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Input/Cursors.cs (C#)

Code before:

namespace Avalonia.Input
{
    /*
    =========================================================================================
        NOTE: Cursors are NOT disposable and are cached in platform implementation.
        To support loading custom cursors some measures about that should be taken beforehand
    =========================================================================================
    */

    public enum StandardCursorType
    {
        Arrow,
        Ibeam,
        Wait,
        Cross,
        UpArrow,
        SizeWestEast,
        SizeNorthSouth,
        SizeAll,
        No,
        Hand,
        AppStarting,
        Help,
        TopSide,
        BottomSide,
        LeftSide,
        RightSide,
        TopLeftCorner,
        TopRightCorner,
        BottomLeftCorner,
        BottomRightCorner,
        DragMove,
        DragCopy,
        DragLink,
        None,

        // Not available in GTK directly, see http://www.pixelbeat.org/programming/x_cursors/ 
        // We might enable them later, preferably, by loading pixmax direclty from theme with fallback image
        // SizeNorthWestSouthEast,
        // SizeNorthEastSouthWest,
    }

    public class Cursor
    {
        public static readonly Cursor Default = new Cursor(StandardCursorType.Arrow);

        internal Cursor(IPlatformHandle platformCursor)
        {
            PlatformCursor = platformCursor;
        }

Review comment (suggestion):
Add
```cs
[Obsolete("Use BottomSide")]
ButtomSize = BottomSide,

```

Code after:

namespace Avalonia.Input
{
    /*
    =========================================================================================
        NOTE: Cursors are NOT disposable and are cached in platform implementation.
        To support loading custom cursors some measures about that should be taken beforehand
    =========================================================================================
    */

    public enum StandardCursorType
    {
        Arrow,
        Ibeam,
        Wait,
        Cross,
        UpArrow,
        SizeWestEast,
        SizeNorthSouth,
        SizeAll,
        No,
        Hand,
        AppStarting,
        Help,
        TopSide,
        BottomSide,
        LeftSide,
        RightSide,
        TopLeftCorner,
        TopRightCorner,
        BottomLeftCorner,
        BottomRightCorner,
        DragMove,
        DragCopy,
        DragLink,
        None,

        [Obsolete("Use BottomSide")]
        BottomSize = BottomSide

        // Not available in GTK directly, see http://www.pixelbeat.org/programming/x_cursors/ 
        // We might enable them later, preferably, by loading pixmax direclty from theme with fallback image
        // SizeNorthWestSouthEast,
        // SizeNorthEastSouthWest,
    }

    public class Cursor
    {
        public static readonly Cursor Default = new Cursor(StandardCursorType.Arrow);

        internal Cursor(IPlatformHandle platformCursor)
Repository: AvaloniaUI/Avalonia
File: tests/Avalonia.Controls.UnitTests/GridTests.cs (C#)

Code before:

            var size = new Size(100, 100);
            grid.Measure(size);
            grid.Arrange(new Rect(size));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            Assert.Equal(100, grid.Children[0].Bounds.Width);

            grid.Children.Add(new Decorator() { Width = 10, Height = 10, [Grid.ColumnProperty] = 1 });

            Assert.False(grid.IsMeasureValid);
            Assert.False(grid.IsArrangeValid);

            grid.Measure(size);
            grid.Arrange(new Rect(size));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            Assert.Equal(90, grid.Children[0].Bounds.Width);
            Assert.Equal(10, grid.Children[1].Bounds.Width);
        }

        private static void Change_Propery_And_Verify_Measure_Requested(Grid grid, Action change)
        {
            grid.Measure(new Size(100, 100));
            grid.Arrange(new Rect(grid.DesiredSize));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            change();

            Assert.False(grid.IsMeasureValid);
            Assert.False(grid.IsArrangeValid);
        }

        private class TestControl : Control
        {
            public Size MeasureSize { get; set; }

            protected override Size MeasureOverride(Size availableSize) => MeasureSize;
        }
    }
}


Review comment (suggestion):
```suggestion
        private static void Change_Property_And_Verify_Measure_Requested(Grid grid, Action change)
```

Code after:

            var size = new Size(100, 100);
            grid.Measure(size);
            grid.Arrange(new Rect(size));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            Assert.Equal(100, grid.Children[0].Bounds.Width);

            grid.Children.Add(new Decorator() { Width = 10, Height = 10, [Grid.ColumnProperty] = 1 });

            Assert.False(grid.IsMeasureValid);
            Assert.False(grid.IsArrangeValid);

            grid.Measure(size);
            grid.Arrange(new Rect(size));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            Assert.Equal(90, grid.Children[0].Bounds.Width);
            Assert.Equal(10, grid.Children[1].Bounds.Width);
        }

        private static void Change_Property_And_Verify_Measure_Requested(Grid grid, Action change)
        {
            grid.Measure(new Size(100, 100));
            grid.Arrange(new Rect(grid.DesiredSize));

            Assert.True(grid.IsMeasureValid);
            Assert.True(grid.IsArrangeValid);

            change();

            Assert.False(grid.IsMeasureValid);
            Assert.False(grid.IsArrangeValid);
        }

        private class TestControl : Control
        {
            public Size MeasureSize { get; set; }

            protected override Size MeasureOverride(Size availableSize) => MeasureSize;
        }
    }
}
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Controls/Converters/PlatformKeyGestureConverter.cs (C#)

Code before:
﻿using System;
using System.Globalization;
using System.Runtime.InteropServices;
using System.Text;
using Avalonia.Data.Converters;
using Avalonia.Input;

namespace Avalonia.Controls.Converters
{
    public class PlatformKeyGestureConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
            if (value is null)
            {
                return null;
            }
            else if (value is KeyGesture gesture && targetType == typeof(string))
            {
                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
                {
                    return ToString(gesture, "Win");
                }
                else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
                {
                    return ToString(gesture, "Super");
                }
                else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
                {
                    return ToOSXString(gesture);
                }
                else
                {
                    return gesture.ToString();
                }
            }
            else
            {
                throw new NotSupportedException();

Review comment (refactor):
Wondering if that could be exposed as a static helper since I would like to reuse that logic in our app and I would need to create a new instance of this converter to do so otherwise (I can obviously use a static shared instance but that feels hacky).

Code after:
﻿using System;
using System.Globalization;
using System.Runtime.InteropServices;
using System.Text;
using Avalonia.Data.Converters;
using Avalonia.Input;

namespace Avalonia.Controls.Converters
{
    /// <summary>
    /// Converts a <see cref="KeyGesture"/> to a string, formatting it according to the current
    /// platform's style guidelines.
    /// </summary>
    public class PlatformKeyGestureConverter : IValueConverter
    {
        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
            if (value is null)
            {
                return null;
            }
            else if (value is KeyGesture gesture && targetType == typeof(string))
            {
                return ToPlatformString(gesture);
            }
            else
            {
                throw new NotSupportedException();
            }
        }

        public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
        {
            throw new NotImplementedException();
        }

        /// <summary>
        /// Converts a <see cref="KeyGesture"/> to a string, formatting it according to the current
        /// platform's style guidelines.
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Animation/Easing/SplineEasing.cs (C#)

Code before:
        }

        public SplineEasing(double x1 = 0d, double y1 = 0d, double x2 = 1d, double y2 = 1d)
        {
            this._internalKeySpline = new KeySpline();
            this.X1 = x1;
            this.Y1 = y1;
            this.X2 = x2;
            this.Y1 = y2;
        }
        
        private KeySpline _internalKeySpline;
        private double _x1;
        private double _y1;
        private double _x2 = 1.0d;
        private double _y2 = 1.0d;
        
        /// <summary>
        /// X coordinate of the first control point
        /// </summary>
        public double X1
        {
            get => _x1;
            set
            {
                _x1 = value; _internalKeySpline.ControlPointX1 = _x1;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point
        /// </summary>
        public double Y1
        {
            get => _y1;
            set
            {
                _y1 = value; _internalKeySpline.ControlPointY1 = _y1;
            }
        }

        /// <summary>
        /// X coordinate of the second control point
        /// </summary>
        public double X2
        {
            get => _x2;
            set
            {
                _x2 = value;
                _internalKeySpline.ControlPointX2 = _x2;

Review comment (suggestion):
You are putting multiple statements in a single line, and it is not even consistent with your own code below which puts same constructs in multiple ones (`X2 for example) 

And I think having multiple statements on the same line is really bad.

Code after:
        public double X1
        {
            get => _internalKeySpline.ControlPointX1;
            set
            {
                _internalKeySpline.ControlPointX1 = value;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point
        /// </summary>
        public double Y1
        {
            get => _internalKeySpline.ControlPointY1;
            set
            {
                _internalKeySpline.ControlPointY1 = value;
            }
        }

        /// <summary>
        /// X coordinate of the second control point
        /// </summary> 
        public double X2
        {
            get => _internalKeySpline.ControlPointX2;
            set
            {
                _internalKeySpline.ControlPointX2 = value;
            }
        }

        /// <summary>
        /// Y coordinate of the second control point
        /// </summary>
        public double Y2
        {
            get => _internalKeySpline.ControlPointY2;
            set
            {
                _internalKeySpline.ControlPointY2 = value;
            }
        }

        private readonly KeySpline _internalKeySpline;

        public SplineEasing(double x1 = 0d, double y1 = 0d, double x2 = 1d, double y2 = 1d)
        {
            _internalKeySpline = new KeySpline();
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Animation/Easing/SplineEasing.cs (C#)

Code before:
namespace Avalonia.Animation.Easings
{
    /// <summary>
    /// Eases a <see cref="double"/> value
    /// using a user-defined cubic bezier curve.
    /// Good for custom easing functions that doesn't quite
    /// fit with the built-in ones. 
    /// </summary>
    public class SplineEasing : Easing
    {
        public SplineEasing()
        {
            this._internalKeySpline = new KeySpline();
        }

        public SplineEasing(double x1 = 0d, double y1 = 0d, double x2 = 1d, double y2 = 1d)
        {
            this._internalKeySpline = new KeySpline();
            this.X1 = x1;
            this.Y1 = y1;
            this.X2 = x2;
            this.Y1 = y2;
        }
        
        private KeySpline _internalKeySpline;
        private double _x1;
        private double _y1;
        private double _x2 = 1.0d;
        private double _y2 = 1.0d;
        
        /// <summary>
        /// X coordinate of the first control point
        /// </summary>
        public double X1
        {
            get => _x1;
            set
            {
                _x1 = value; _internalKeySpline.ControlPointX1 = _x1;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point

Review comment (suggestion):
I also wonder if we need dedicated storage for the control points since you might as well just use `KeySpline` members to save memory.

Code after:
namespace Avalonia.Animation.Easings
{
    /// <summary>
    /// Eases a <see cref="double"/> value
    /// using a user-defined cubic bezier curve.
    /// Good for custom easing functions that doesn't quite
    /// fit with the built-in ones. 
    /// </summary>
    public class SplineEasing : Easing
    {
        /// <summary>
        /// X coordinate of the first control point
        /// </summary>
        public double X1
        {
            get => _internalKeySpline.ControlPointX1;
            set
            {
                _internalKeySpline.ControlPointX1 = value;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point
        /// </summary>
        public double Y1
        {
            get => _internalKeySpline.ControlPointY1;
            set
            {
                _internalKeySpline.ControlPointY1 = value;
            }
        }

        /// <summary>
        /// X coordinate of the second control point
        /// </summary> 
        public double X2
        {
            get => _internalKeySpline.ControlPointX2;
            set
            {
                _internalKeySpline.ControlPointX2 = value;
            }
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Animation/Easing/SplineEasing.cs (C#)

Code before:
    /// <summary>
    /// Eases a <see cref="double"/> value
    /// using a user-defined cubic bezier curve.
    /// Good for custom easing functions that doesn't quite
    /// fit with the built-in ones. 
    /// </summary>
    public class SplineEasing : Easing
    {
        public SplineEasing()
        {
            this._internalKeySpline = new KeySpline();
        }

        public SplineEasing(double x1 = 0d, double y1 = 0d, double x2 = 1d, double y2 = 1d)
        {
            this._internalKeySpline = new KeySpline();
            this.X1 = x1;
            this.Y1 = y1;
            this.X2 = x2;
            this.Y1 = y2;
        }
        
        private KeySpline _internalKeySpline;
        private double _x1;
        private double _y1;
        private double _x2 = 1.0d;
        private double _y2 = 1.0d;
        
        /// <summary>
        /// X coordinate of the first control point
        /// </summary>
        public double X1
        {
            get => _x1;
            set
            {
                _x1 = value; _internalKeySpline.ControlPointX1 = _x1;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point
        /// </summary>
        public double Y1
        {
            get => _y1;
            set
            {
                _y1 = value; _internalKeySpline.ControlPointY1 = _y1;
            }
        }

Review comment (suggestion):
I find this mighty confusing that certain values are defined here, and some are passed with that all default value ctor (which you can't even call easily call as `new SplineEasing()` invokes parameterless one).

Code after:
    /// <summary>
    /// Eases a <see cref="double"/> value
    /// using a user-defined cubic bezier curve.
    /// Good for custom easing functions that doesn't quite
    /// fit with the built-in ones. 
    /// </summary>
    public class SplineEasing : Easing
    {
        /// <summary>
        /// X coordinate of the first control point
        /// </summary>
        public double X1
        {
            get => _internalKeySpline.ControlPointX1;
            set
            {
                _internalKeySpline.ControlPointX1 = value;
            }
        }

        /// <summary>
        /// Y coordinate of the first control point
        /// </summary>
        public double Y1
        {
            get => _internalKeySpline.ControlPointY1;
            set
            {
                _internalKeySpline.ControlPointY1 = value;
            }
        }

        /// <summary>
        /// X coordinate of the second control point
        /// </summary> 
        public double X2
        {
            get => _internalKeySpline.ControlPointX2;
            set
            {
                _internalKeySpline.ControlPointX2 = value;
            }
        }

        /// <summary>
        /// Y coordinate of the second control point
        /// </summary>
        public double Y2
        {
            get => _internalKeySpline.ControlPointY2;
            set
Repository: AvaloniaUI/Avalonia
File: src/Avalonia.Base/Input/GestureRecognizers/GestureRecognizer.cs (C#)

Code before:
﻿using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Avalonia.Input.GestureRecognizers
{
    public abstract class GestureRecognizer : StyledElement
    {
        public abstract IInputElement? Target { get; }
        public abstract void Initialize(IInputElement target);
        public abstract void PointerPressed(PointerPressedEventArgs e);
        public abstract void PointerReleased(PointerReleasedEventArgs e);
        public abstract void PointerMoved(PointerEventArgs e);
        public abstract void PointerCaptureLost(IPointer pointer);

        protected void Capture(IPointer pointer)
        {
            (pointer as Pointer)?.CaptureGestureRecognizer(this);
        }
    }
}


Review comment (suggestion):
Both `Target` and `Initialize` should be handled by the base class, IMO. 

e. g. 
```
protected internal IInputElement? Target { get; internal set; }
```
with `Initialize` being removed completely.

Code after:
﻿namespace Avalonia.Input.GestureRecognizers
{
    public abstract class GestureRecognizer : StyledElement
    {
        protected internal IInputElement? Target { get; internal set; }

        protected abstract void PointerPressed(PointerPressedEventArgs e);
        protected abstract void PointerReleased(PointerReleasedEventArgs e);
        protected abstract void PointerMoved(PointerEventArgs e);
        protected abstract void PointerCaptureLost(IPointer pointer);

        internal void PointerPressedInternal(PointerPressedEventArgs e)
        {
            PointerPressed(e);
        }

        internal void PointerReleasedInternal(PointerReleasedEventArgs e)
        {
            PointerReleased(e);
        }

        internal void PointerMovedInternal(PointerEventArgs e)
        {
            PointerMoved(e);
        }

        internal void PointerCaptureLostInternal(IPointer pointer)
        {
            PointerCaptureLost(pointer);
        }

        protected void Capture(IPointer pointer)
        {
            (pointer as Pointer)?.CaptureGestureRecognizer(this);
        }
    }
Repository: AvaloniaUI/Avalonia
File: native/Avalonia.Native/src/OSX/WindowImpl.mm (Objective-C++)

Code before:

void WindowImpl::DoZoom() {
    if (_decorations == SystemDecorationsNone ||
        _decorations == SystemDecorationsBorderOnly ||
        _canResize == false ||
        _canMaximize == false) {
        [Window setFrame:[Window screen].visibleFrame display:true];
    } else {
        [Window performZoom:Window];
    }
}

HRESULT WindowImpl::SetCanResize(bool value) {
    START_COM_CALL;

    @autoreleasepool {
        _canResize = value;
        UpdateAppearance();
        return S_OK;
    }
}

HRESULT WindowImpl::SetCanMinimize(bool value) {
    START_COM_CALL;

    @autoreleasepool {
        _canMinimize = value;
        UpdateAppearance();
        return S_OK;
    }
}

HRESULT WindowImpl::SetCanMaximize(bool value) {
    START_COM_CALL;

    @autoreleasepool {
        _canMaximize = value;
        UpdateAppearance();
        return S_OK;
    }
}

HRESULT WindowImpl::SetDecorations(SystemDecorations value) {
    START_COM_CALL;

    @autoreleasepool {
        auto currentWindowState = _lastWindowState;
        _decorations = value;

        if (_fullScreenActive) {
            return S_OK;

Review comment (suggestion):
1) we probably don't need an ARP when we are on the UI thread, since there should be one provided by the event loop
2) use START_COM_ARP_CALL macro that enables both COM-safeguard and an ARP

Code after:

void WindowImpl::DoZoom() {
    if (_decorations == SystemDecorationsNone ||
        _decorations == SystemDecorationsBorderOnly ||
        _canResize == false ||
        _canMaximize == false) {
        [Window setFrame:[Window screen].visibleFrame display:true];
    } else {
        [Window performZoom:Window];
    }
}

HRESULT WindowImpl::SetCanResize(bool value) {
    START_COM_CALL;

    @autoreleasepool {
        _canResize = value;
        UpdateAppearance();
        return S_OK;
    }
}

HRESULT WindowImpl::SetCanMinimize(bool value) {
    START_COM_ARP_CALL;

    _canMinimize = value;
    UpdateAppearance();
    return S_OK;
}

HRESULT WindowImpl::SetCanMaximize(bool value) {
    START_COM_ARP_CALL;

    _canMaximize = value;
    UpdateAppearance();
    return S_OK;
}

HRESULT WindowImpl::SetDecorations(SystemDecorations value) {
    START_COM_CALL;

    @autoreleasepool {
        auto currentWindowState = _lastWindowState;
        _decorations = value;

        if (_fullScreenActive) {
            return S_OK;
        }

        UpdateAppearance();