Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InputModulesのBuilder #195

Open
H1rono opened this issue May 26, 2024 · 4 comments · May be fixed by #199
Open

InputModulesのBuilder #195

H1rono opened this issue May 26, 2024 · 4 comments · May be fixed by #199
Labels
enhancement New feature or request

Comments

@H1rono
Copy link
Member

H1rono commented May 26, 2024

/**
* @brief Construct a new Input Modules object
* @param joy_pins ジョイスティックのピン2つ
* @param volume_pin ツマミのピン
* @param mpu_pins MPU6050のピン (sda, scl)
*/
InputModules(
const std::pair<PinName, PinName>& joy_pins, const PinName& volume_pin,
const std::pair<PinName, PinName>& mpu_pins);

// NOLINTBEGIN(bugprone-easily-swappable-parameters)
device::InputModules::InputModules(
const std::pair<PinName, PinName>& joy_pins, const PinName& volume_pin,
const std::pair<PinName, PinName>& mpu_pins) :
joy(joy_pins.first, joy_pins.second),
volume(volume_pin),
mpu(mpu_pins.first, mpu_pins.second) {}
// NOLINTEND(bugprone-easily-swappable-parameters)

この部分を↓にしたい

class InputModules {
public:
    class Builder {
    public:
        ...
        auto build() -> InputModules;
    };

private:
    // InputModulesをBuilder以外の方法で構築できないようにする
    InputModules(Builder builder);

public:
    static auto builder() -> Builder;
    ...
};

クラスを入れ子にするあたりが怪しいかも?要調査

@H1rono H1rono added the enhancement New feature or request label May 27, 2024
@23-yoshikawa
Copy link
Contributor

はじめの...に
std::pair<PinName, PinName>& joy_pins;;
PinName& volume_pin,;
std::pair<PinName, PinName>& mpu_pins;
を入れて、
2つめの...に
InputModules(
const std::pair<PinName, PinName>& joy_pins, const PinName& volume_pin,
const std::pair<PinName, PinName>& mpu_pins);
を入れて、
.cppファイルのほうでbuild()、builder()の関数を実装すればいいのでしょうか。

@H1rono
Copy link
Member Author

H1rono commented May 30, 2024

そもそも変更後はこのように使用することが目的です ( #192 )

InputModules input_modules = InputModules::builder()
    .joy_pins(A4, A5)
    .volume_pin(A6)
    .mpu_sda_pin(D4)
    .mpu_scl_pin(D5)
    .build();

// こう書いても同じ
InputModules::Builder builder = InputModules::builder();
builder.joy_pins(A4, A5);
builder.volume_pin(A6);
builder.mpu_sda_pin(D4);
builder.scl_pin(D5);
InputModules input = builder.build();

はじめの...

Builderは内部状態として確かにそれを持っておく必要があるんですが、メソッドチェーンできるようにするためにSetterをかます必要があります。

class Builder {
private:
    PinName _volume_pin;
    ...

public:
    auto volume_pin(PinName pin) -> Builder&;
    ...
};

2つめの...

擬似コードのコメントにも書きましたが、InputModulesBuilder以外から構築できないようにしたいです。そのため、↓のようなコンストラクタはそもそも作らないか、private指定してInputModulesの外からは見えないようにしたいです。

InputModules(
    const std::pair<PinName, PinName>& joy_pins,
    const PinName& volume_pin,
    const std::pair<PinName, PinName>& mpu_pins,
);

あとこれは擬似コードが悪いんですが、Builderを受け取るコンストラクタはpublicにしても良さそう

2つめの...にはそれ以外で既存のメソッドたちが入るはずです。

InputModules() = delete;
~InputModules() = default;
InputModules(const InputModules&) = delete;
auto operator=(const InputModules&) -> InputModules& = delete;
InputModules(InputModules&&) = default;
auto operator=(InputModules&&) -> InputModules& = default;
/**
* @brief MPU6050のWHOAMI `MPU6050::testConnection` 参照
* @return true success
* @return false failure
*/
auto mpu_whoami() -> bool;
/**
* @brief センサー類の値を読む
* @return packet::InputValues 読んだ値まとめ
*/
auto read() -> packet::InputValues;

.cppファイルのほうで...

はい、それに加えてBuilderの持つ各種setterを実装する必要があります

@H1rono
Copy link
Member Author

H1rono commented May 30, 2024

擬似コード書き直し

class InputModules {
public:
    class Builder {
    private:
        PinName _volume_pin;
        // 他のピンも書く

    public:
        auto volume_pin(const PinName& pin) -> Builder&;
        // その他setterも
        auto build() -> InputModules;
    };

private:
    // あってもなくてもいい
    InputModules(
        const std::pair<PinName, PinName>& joy_pins,
        const PinName& volume_pin,
        const std::pair<PinName, PinName>& mpu_pins
    );

public:
    static auto builder() -> Builder;
    // InputModulesをBuilder以外の方法で構築できないようにする
    // すなわち、publicなコンストラクタはこれだけ
    InputModules(Builder builder);
    // 元々あったメソッドたち
};

@23-yoshikawa
Copy link
Contributor

丁寧な解説ありがとうございます。
やってみます

@H1rono H1rono linked a pull request Jun 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants