CODEWITHSUNDEEP
c++design-patternsc++20builder
senloa
senloa

Reputation: 311

Builder pattern with struct

I'm trying to implement a builder patter in C++. That what i got by now:

struct Person {
  std::string name;
  uint32_t age;
  std::vector<std::string> pet_names;
};


class PersonBuilder {
public:
  PersonBuilder& SetName(std::string name) {
    person_.name = std::move(name);
    return *this;
  }

  PersonBuilder& SetAge(uint32_t age) {
    person_.age = age;
    return *this;
  }

  PersonBuilder& SetPetNames(std::vector<std::string> pet_names) {
    person_.pet_names = std::move(pet_names);
    return *this;
  }

  Person Build() {
    return Person(std::move(person_));
  }

private:
  Person person_;
};

int main() {
  auto person = PersonBuilder()
      .SetName("John")
      .SetAge(23)
      .SetPetNames({"rax", "rbx", "rcx"})
      .Build();

  return EXIT_SUCCESS;
}

I'm a little worried about unitilized uint32, and std::move(person_) inside a Build method. Am i doing it right or there is a better way, with a struct constructor for example?

Upvotes: 4

Views: 1160

Answers (2)

康桓瑋
康桓瑋

Reputation: 42861

You did it right.

You can also turn the set function into a template function, which can avoid the modification of PersonBuilder when adding new member variables for Person.

struct Person {
  std::string name;
  uint32_t age;
  std::vector<std::string> pet_names;
};

class PersonBuilder {
public:
  template<auto mem_ptr>
  PersonBuilder& Set(std::remove_reference_t<std::invoke_result_t<decltype(mem_ptr), Person&>> value = {}) {
    std::invoke(mem_ptr, person_) = std::move(value);
    return *this;
  }

  Person Build() {
    auto result = std::move(person_); 
    person_ = Person(); 
    return result;
  }

private:
  Person person_;
};

int main() {
  auto person = PersonBuilder()
    .Set<&Person::name>("John")
    .Set<&Person::age>(23)
    .Set<&Person::pet_names>({"rax", "rbx", "rcx"})
    .Build();
}

Demo.

Upvotes: 3

eerorika
eerorika

Reputation: 238391

I'm a little worried about unitilized uint32

You initialise it before you use it, so there is no problem in the example. It is however easy for a user of the builder to forget calling one of the setters leaving it with indeterminate value resulting in undefined behaviour. You give it a default member initialiser to avoid that.

and std::move(person_) inside a Build method.

It's unnecessary to repeat the name of the class there. This would also work:

return std::move(person_);

It's also important for the caller to understand that Build may only be called once per initialisation. If the builder is intended to only be used as a temporary like in the example, you could add rvalue reference qualifier to the Build function to prevent some of the accidental misuse.

or there is a better way,

I don't see the benefit of the builder. Why not just use aggregate initialisation?:

Person person {
    .name = "John",
    .age = 23,
    .pet_names = {"rax", "rbx", "rcx"},
};

Upvotes: 3

Related Questions