ronantakizawa/codereview-bench huggingface dataset:
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}


Review comment (suggestion):
I feel like having two separate functions to do almost the same thing is kind of redundant. You could instead have one function that takes the value to add or remove as parameter, then clamp the value to 0-100 range.

Code after:
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun changeOpacity(delta : Int) {
        controls.allButtons.forEach {
            val newOpacity = (it.config.opacity + delta).coerceIn(0, 100)
            it.config.opacity = newOpacity
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
        }
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
    init {
        setOnTouchListener(playingTouchHandler)
    }

    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}


Review comment (suggestion):
```suggestion
    fun changeOpacity(delta : Int) {
```


Code after:
    init {
        setOnTouchListener(playingTouchHandler)
    }

    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun changeOpacity(delta : Int) {
        controls.allButtons.forEach {
            val newOpacity = (it.config.opacity + delta).coerceIn(0, 100)
            it.config.opacity = newOpacity
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
        }
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}


Review comment (suggestion):
```suggestion
            it.config.opacity = (it.config.opacity + delta).coerceIn(0, 100)
```

Code after:
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun changeOpacity(delta : Int) {
        controls.allButtons.forEach {
            val newOpacity = (it.config.opacity + delta).coerceIn(0, 100)
            it.config.opacity = newOpacity
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
        }
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}


Review comment (suggestion):
```suggestion
            if (it is JoystickButton) it.innerButton.config.opacity = newOpacity
```

Code after:
    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun changeOpacity(delta : Int) {
        controls.allButtons.forEach {
            val newOpacity = (it.config.opacity + delta).coerceIn(0, 100)
            it.config.opacity = newOpacity
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
        }
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}


Review comment (suggestion):
```suggestion
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
```
As per our codestyle guidelines, we don't do single line `if` statements. We just leave out curly brackets but keep the newline.

Code after:
    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += 0.05f
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= 0.05f
        invalidate()
    }

    fun changeOpacity(delta : Int) {
        controls.allButtons.forEach {
            val newOpacity = (it.config.opacity + delta).coerceIn(0, 100)
            it.config.opacity = newOpacity
            if (it is JoystickButton) 
                it.innerButton.config.opacity = newOpacity
        }
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled
        invalidate()
    }
}
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/input/onscreen/OnScreenControllerView.kt (Kotlin)

Code before:
            false
        }.also { handled -> if (handled) invalidate() }
    }

    init {
        setOnTouchListener(playingTouchHandler)
    }

    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += SCALE_STEP
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= SCALE_STEP
        invalidate()
    }

    fun increaseOpacity() {
        controls.alpha = (controls.alpha + ALPHA_STEP).coerceIn(ALPHA_RANGE)
        invalidate()
    }

    fun decreaseOpacity() {
        controls.alpha = (controls.alpha - ALPHA_STEP).coerceIn(ALPHA_RANGE)
        invalidate()
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }

    fun setOnStickStateChangedListener(listener : OnStickStateChangedListener) {
        onStickStateChangedListener = listener
    }

    fun getButtonProps() = controls.allButtons.map { Pair(it.buttonId, it.config.enabled) }

    fun setButtonEnabled(buttonId : ButtonId, enabled : Boolean) {
        controls.allButtons.first { it.buttonId == buttonId }.config.enabled = enabled

Review comment (suggestion):
`getBackgroundColor`, don't shorten to BG. Swap out BG for background also for all other functions in this and in other files.

Code after:
            false
        }.also { handled -> if (handled) invalidate() }
    }

    init {
        setOnTouchListener(playingTouchHandler)
    }

    fun setEditMode(editMode : Boolean) = setOnTouchListener(if (editMode) editingTouchHandler else playingTouchHandler)

    fun resetControls() {
        controls.allButtons.forEach {
            it.resetRelativeValues()
            it.config.enabled = true
        }
        controls.globalScale = 1.15f
        controls.alpha = 155
        invalidate()
    }

    fun increaseScale() {
        controls.globalScale += SCALE_STEP
        invalidate()
    }

    fun decreaseScale() {
        controls.globalScale -= SCALE_STEP
        invalidate()
    }

    fun increaseOpacity() {
        controls.alpha = (controls.alpha + ALPHA_STEP).coerceIn(ALPHA_RANGE)
        invalidate()
    }

    fun decreaseOpacity() {
        controls.alpha = (controls.alpha - ALPHA_STEP).coerceIn(ALPHA_RANGE)
        invalidate()
    }

    fun getTextColor() : Int {
        return controls.globalTextColor
    }

    fun getBackGroundColor() : Int {
        return controls.globalBackgroundColor
    }

    fun setOnButtonStateChangedListener(listener : OnButtonStateChangedListener) {
        onButtonStateChangedListener = listener
    }
Repository: skyline-emu/skyline
File: app/src/main/java/emu/skyline/data/DataItem.kt (Kotlin)

Code before:
/**
 * This class is a wrapper around [AppEntry], it is used for passing around game metadata
 */
data class AppItem(private val meta : AppEntry) : DataItem() {
    /**
     * The icon of the application
     */
    val icon get() = meta.icon

    /**
     * The title of the application
     */
    val title get() = meta.name

    /**
     * The string used as the sub-title, we currently use the author
     */
    val subTitle get() = meta.author

    /**
     * The URI of the application's image file
     */
    val uri get() = meta.uri

    val loaderResult get() = meta.loaderResult

    fun loaderResultString(context : Context) = context.getString(when (meta.loaderResult) {
        LoaderResult.Success -> R.string.metadata_missing

        LoaderResult.ParsingError -> R.string.invalid_file

        LoaderResult.MissingTitleKey -> R.string.missing_title_key

        LoaderResult.MissingHeaderKey,
        LoaderResult.MissingTitleKek,
        LoaderResult.MissingKeyArea -> R.string.incomplete_prod_keys
    })

    /**
     * The name and author is used as the key
     */
    fun key() = meta.name + if (meta.author != null) " ${meta.author}" else ""
}


Review comment (security):
```suggestion
     * The string used as the application version, we currently use the author
```

Code after:
/**
 * This class is a wrapper around [AppEntry], it is used for passing around game metadata
 */
data class AppItem(private val meta : AppEntry) : DataItem() {
    /**
     * The icon of the application
     */
    val icon get() = meta.icon

    /**
     * The title of the application
     */
    val title get() = meta.name

    /**
     * The string used as the sub-title, we currently use the author
     */
    val subTitle get() = meta.author

    /**
     * The URI of the application's image file
     */
    val uri get() = meta.uri

    /**
     * The application version
     */
    val version get() = meta.version

    val loaderResult get() = meta.loaderResult

    fun loaderResultString(context : Context) = context.getString(when (meta.loaderResult) {
        LoaderResult.Success -> R.string.metadata_missing

        LoaderResult.ParsingError -> R.string.invalid_file

        LoaderResult.MissingTitleKey -> R.string.missing_title_key

        LoaderResult.MissingHeaderKey,
        LoaderResult.MissingTitleKek,
        LoaderResult.MissingKeyArea -> R.string.incomplete_prod_keys
    })

    /**
     * The name and author is used as the key
     */
    fun key() = meta.name + if (meta.author != null) " ${meta.author}" else ""
}
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/input/npad_device.h (C/C++)

Code before:
// Copyright © 2020 Skyline Team and Contributors (https://github.com/skyline-emu/)

#pragma once

#include <kernel/types/KEvent.h>
#include "shared_mem.h"

namespace skyline::input {
    /**
     * @brief How many joycons must be attached for handheld mode to be triggered
     */
    enum class NpadHandheldActivationMode : u64 {
        Dual = 0,
        Single = 1,
        None = 2,
    };

    /**
     * @brief The orientations the Joy-Con(s) can be held in
     */
    enum class NpadJoyOrientation : i64 {
        Vertical = 0,
        Horizontal = 1,
    };

    /**
     * @url https://switchbrew.org/wiki/HID_services#NpadStyleTag
     */
    union NpadStyleSet {
        u32 raw;
        struct {
            bool proController : 1; //!< Pro Controller
            bool joyconHandheld : 1; //!< Joy-Cons in handheld mode
            bool joyconDual : 1; //!< Joy-Cons in a pair
            bool joyconLeft : 1; //!< Left Joy-Con only
            bool joyconRight : 1; //!< Right Joy-Con only
            bool gamecube : 1; //!< GameCube controller
            bool palma : 1; //!< Poké Ball Plus controller
            bool nes : 1; //!< NES controller
            bool nesHandheld : 1; //!< NES controller in handheld mode
            bool snes : 1; //!< SNES controller
        };
    };
    static_assert(sizeof(NpadStyleSet) == 0x4);

    enum class NpadAxisId {
        LX, //!< Left Stick X
        LY, //!< Left Stick Y
        RX, //!< Right Stick X
        RY, //!< Right Stick Y
    };

Review comment (suggestion):
```suggestion
        std::array<float, 3> gyroscope;
```
(for all)

Code after:
// Copyright © 2020 Skyline Team and Contributors (https://github.com/skyline-emu/)

#pragma once

#include <kernel/types/KEvent.h>
#include "shared_mem.h"

namespace skyline::input {

    /**
     * @brief Motion sensor location
     */
    enum class MotionId {
        Left,
        Right,
        Console,
    };

    /**
     * @brief A description of a motion event
     * @note This structure corresponds to MotionSensorInput, see that for details
     */
    struct MotionSensorState {
        u64 timestamp;
        u64 deltaTimestamp;
        std::array<float,3> gyroscope;
        std::array<float,3> accelerometer;
        std::array<float,4> quaternion;
        std::array<float,9> orientationMatrix;
    };
    static_assert(sizeof(MotionSensorState) == 0x60);

    /**
     * @brief How many joycons must be attached for handheld mode to be triggered
     */
    enum class NpadHandheldActivationMode : u64 {
        Dual = 0,
        Single = 1,
        None = 2,
    };

    /**
     * @brief The orientations the Joy-Con(s) can be held in
     */
    enum class NpadJoyOrientation : i64 {
        Vertical = 0,
        Horizontal = 1,
    };

    /**
     * @url https://switchbrew.org/wiki/HID_services#NpadStyleTag
Repository: skyline-emu/skyline
File: app/src/main/cpp/skyline/applet/error_applet.h (C/C++)

Code before:
        };
        static_assert(sizeof(ErrorCommonHeader) == 0x8);

        struct ApplicationErrorArg {
            ErrorCommonHeader commonHeader;
            u32 errorNumber;
            LanguageCode languageCode;
            std::array<char, 0x800> dialogMessage;
            std::array<char, 0x800> fullscreenMessage; //!< The message displayed when the user clicks on "Details", when not set this disables displaying Details
        };
        static_assert(sizeof(ApplicationErrorArg) == 0x1014);

        #pragma pack(pop)

        std::shared_ptr<service::am::IStorage> errorStorage;

        void HandleApplicationErrorArg();

      public:
        ErrorApplet(const DeviceState &state, service::ServiceManager &manager, std::shared_ptr<kernel::type::KEvent> onAppletStateChanged, std::shared_ptr<kernel::type::KEvent> onNormalDataPushFromApplet, std::shared_ptr<kernel::type::KEvent> onInteractiveDataPushFromApplet, service::applet::LibraryAppletMode appletMode);

        Result Start() override;

        Result GetResult() override;

        void PushNormalDataToApplet(std::shared_ptr<service::am::IStorage> data) override;

        void PushInteractiveDataToApplet(std::shared_ptr<service::am::IStorage> data) override;
    };
}


Review comment (suggestion):
```suggestion
        void HandleErrorCommonArg();
```


Code after:
        };
        static_assert(sizeof(ErrorCommonHeader) == 0x8);

        /**
         * @url https://switchbrew.org/wiki/Error_Applet#ErrorCommonArg
         */
        struct ErrorCommonArg {
            ErrorCommonHeader header;
            u64 errorCode;
            Result result;
        };

        struct ApplicationErrorArg {
            ErrorCommonHeader commonHeader;
            u32 errorNumber;
            LanguageCode languageCode;
            std::array<char, 0x800> dialogMessage;
            std::array<char, 0x800> fullscreenMessage; //!< The message displayed when the user clicks on "Details", when not set this disables displaying Details
        };
        static_assert(sizeof(ApplicationErrorArg) == 0x1014);

        #pragma pack(pop)

        std::shared_ptr<service::am::IStorage> errorStorage;

        void HandleErrorCommonArg();

        void HandleApplicationErrorArg();

      public:
        ErrorApplet(const DeviceState &state, service::ServiceManager &manager, std::shared_ptr<kernel::type::KEvent> onAppletStateChanged, std::shared_ptr<kernel::type::KEvent> onNormalDataPushFromApplet, std::shared_ptr<kernel::type::KEvent> onInteractiveDataPushFromApplet, service::applet::LibraryAppletMode appletMode);

        Result Start() override;

        Result GetResult() override;

        void PushNormalDataToApplet(std::shared_ptr<service::am::IStorage> data) override;

        void PushInteractiveDataToApplet(std::shared_ptr<service::am::IStorage> data) override;
    };
}