rogy-AquaLab / omniboat_robokit

新歓で使うロボキット
https://rogy-aqualab.github.io/omniboat_robokit/
Apache License 2.0
0 stars 0 forks source link

InputModulesのBuilder #195

Open H1rono opened 4 months ago

H1rono commented 4 months ago

https://github.com/rogy-AquaLab/omniboat_robokit/blob/2d3b5b3c458545a219c165dd51108eb6995a1f62/include/device/input.hpp#L33-L41

https://github.com/rogy-AquaLab/omniboat_robokit/blob/2d3b5b3c458545a219c165dd51108eb6995a1f62/src/device/input.cpp#L13-L20

この部分を↓にしたい

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

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

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

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

23-yoshikawa commented 4 months ago

はじめの...に 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 commented 4 months ago

そもそも変更後はこのように使用することが目的です ( #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つめの...にはそれ以外で既存のメソッドたちが入るはずです。

https://github.com/rogy-AquaLab/omniboat_robokit/blob/46c1b3c106126ec64cb2e368ba16c74bd0caca5a/include/device/input.hpp#L43-L61

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

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

H1rono commented 4 months ago

擬似コード書き直し

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 commented 4 months ago

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