14

This question asks for a clean way of implementing a static factory method in C++, and this answer describes a clear way to do so. Return Value Optimization would save us from making an unnecesary copy of Object, thus making this way of creating an Object as efficient as directly invoking a constructor. The overhead of copying i to id inside a private constructor is negligible because it's a small int.

However, the question and answer don't cover a more complex case when Object contains an instance variable that is an instance of class Foo (that requires complex initialization logic) rather than a small primitive type. Suppose I want to construct Foo using the arguments passed to Object. A solution using a constructor would look something like:

class Object {
    Foo foo;

public:
    Object(const FooArg& fooArg) {
        // Create foo using fooArg here
        foo = ...
    }
}

An alternative with a static factory method analogous to the quoted answer would be, as it appears to me:

class Object {
    Foo foo;

    explicit Object(const Foo& foo_):
        foo(foo_)
    {

    }

public:
    static Object FromFooArg(const FooArg& fooArg) {
        // Create foo using fooArg here
        Foo foo = ...
        return Object(foo);
    }
}

Here, the overhead of copying foo_ to foo is no longer necessarily negligible, since Foo can be an arbitrarily complex class. Moreover, as far as I understand (C++ newbie here so I may be wrong), this code implicitly requires for a copy constructor to be defined for Foo.

What would be a similarly clean but also efficient way of implementing this pattern in this case?

To anticipate possible questions about why this is relevant, I consider having constructors with logic more complicated than just copying the arguments to be an anti-pattern. I expect the constructor to:

  • be guaranteed to work and not throw exceptions,
  • and not do heavy calculations under the hood.

Thus, I prefer to put complex initialization logic into static methods. Moreover, this approach provides additional benefits such as overloading by static factory method name even when the input argument types are the same, and the possibility of clearly stating what is being done inside in the name of the method.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
Vossler
  • 243
  • 2
  • 6
  • 2
    Since C++11 you do have *move semantics* available to you. But it's a complex topic; best answered by a good book like Stroustrup. Nicely written question though; +1. – Bathsheba Jun 20 '18 at 06:59
  • Strictly speaking, your first version with the public c'tor is expensive also. You default initialize `foo` (I assume all initialization is expensive), then you create *another* `Foo` object, and assign it to the default constructed one. – StoryTeller - Unslander Monica Jun 20 '18 at 07:03
  • @StoryTeller Hmm, I had no idea that was necessarily the case. I thought that in the first version, if I do `foo = Foo(fooArg)`, then only that constructor gets invoked and no copying occurs. My knowledge of C++ is really shallow jumping in from Java. Anyway, my main applied concern is about _copying_ rather than _initialization_ (which can be cheap even for potentially large objects, e.g. empty array initialization). – Vossler Jun 20 '18 at 07:13
  • 1
    @Vossler - I'd advise to tread carefully here. The C++ object model is very different to Java's. Initialization of members and copying are not quite as disjoint as they are in Java. – StoryTeller - Unslander Monica Jun 20 '18 at 07:16
  • What is wrong with a constructor that *initializes* its `foo` directly from a `fooArg`? Such a constructor can, of course, also be perused by a factory. – Walter Jun 20 '18 at 08:53

2 Answers2

7

Thanks to move constructor, you might do:

class Object {
    Foo foo;

    explicit Object(Foo&& foo_) : foo(std::move(foo_)) {}

public:
    static Object FromFooArg(const FooArg& fooArg) {
        // Create foo using fooArg here
        Foo foo = ...
        return Object(std::move(foo));
    }
};

If Foo is not movable, wrapping it in smart pointer is a possibility:

class Object {
    std::unique_ptr<Foo> foo;

    explicit Object(std::unique_ptr<Foo>&& foo_) : foo(std::move(foo_)) {}

public:
    static Object FromFooArg(const FooArg& fooArg) {
        // Create foo using fooArg here
        std::unique_ptr<Foo> foo = ...
        return Object(std::move(foo));
    }
};
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • Thanks! I am probably biased towards these solutions b/c this more or less imitates Java pass-by-reference style, which is more familiar to me. Two questions about this: 1) a closer Java clone would be using `std::shared_ptr` instead of `std::unique_ptr` in solution 2, right? And then we kind of imitate Java's reference-counting garbage collection? 2) while this feels good, the verbosity and complexity of such a simple operation feels off. If I follow this idiom generally, I would then have to wrap everything into smart pointers, right? _Am I doing it wrong in C++_? – Vossler Jun 20 '18 at 10:34
  • Well, C++ is not Java. Using `shared_ptr` all over the place incurs a cost and also makes it difficult to reason about deterministic destruction. – dandan78 Jun 20 '18 at 11:15
  • @Vossler: garbage collection != reference counting, but yes using `shared_ptr` would be more like Java. – Jarod42 Jun 20 '18 at 12:03
  • Why do you need a heap allocation if you have [copy elision](https://en.cppreference.com/w/cpp/language/copy_elision) to do the job for you? If `Foo` is expensive to construct you will pay the cost once. – Jorge Bellon Jun 20 '18 at 12:13
5

What is wrong with initializing the instance in the constructor directly from the arguments needed to do so?

class Object
{
    Foo foo;                         // or const Foo foo, disallowing assignment

public:

    explicit Object(FooCtorArgs const&fooArg,
                    const AdditionalData*data = nullptr)
      : foo(fooArg)                  // construct instance foo directly from args
    {
        foo.post_construction(data); // optional; doesn't work with const foo
    }

    static Object FromFooArg(FooCtorArgs const&fooArg,
                             const AdditionalData*data = nullptr)
    { 
        return Object{fooArg,data};  // copy avoided by return value optimization
    }
};

AFAICT, there is no need to copy/move anything, even if you need to adjust foo post construction.

Walter
  • 44,150
  • 20
  • 113
  • 196
  • While this is useful to me, this only works in the particular case when `Foo` has a constructor directly from `FooArg` (right?..). If, instead, in `FromFooArg` I default-initialize `foo` and fill it manually using some computations from `fooArg` (or other arguments in the general case), then you would have to fall back to my original snippet. Besides, this solution kind of soft-violates my motivation of not having constructors doing complex initialization logic (it depends on `FooArg` also adhering to this convention). – Vossler Jun 20 '18 at 10:22
  • That concern is a red herring. In this case, you can just fill in the `Object::foo` instance, either in the factory or in the constructor of `Object`, see edit. However, a well designed `class Foo` should not need such post-construction initialization. – Walter Jun 20 '18 at 10:45
  • @ÖöTiib Please read my original question - neither the text nor the code indicate that I assume `Foo` has a constructor directly from `FooArg`. If you consider the original question to be poorly formulated, then you are justified in downvoting it, but your implication that my comment to this answer enforces some ad-hoc constraint on `Foo` is wrong. – Vossler Jun 20 '18 at 10:54
  • @Vossler How else? Your `Object` is constructible from `FooArg` only and has `Foo` component so that clearly implies that `Foo` is constructible from `FooArg` only as well. Otherwise information does flow in sideways from some unsaid-out dependencies. – Öö Tiib Jun 20 '18 at 10:59
  • @ÖöTiib `Foo` is indeed constructible from `FooArg`, yet there does not have to be a direct constructor defined. Who said I have control over the implementation of `Foo`? – Vossler Jun 20 '18 at 11:07
  • @Vossler but why you bang on "directly", what is the difference if `Foo` constructor takes `fooArg` argument or `fooArg.gimmeSomething()` argument? It will be still constructed at place. OTOH when way from `FooArg` to `Object` is very complex then that is usually done in "factory" or "builder" before constructing an `Object` because constructor is unsuitable for complex error-handling for obvious reasons. – Öö Tiib Jun 20 '18 at 11:20
  • @ÖöTiib this (equivalence of `fooArg` and `fooArg.gimmeSomething()`) is a valid point that I didn't see before for some reason, and is a good answer to my original comment to this answer. Doesn't mean I added ad-hoc limitations to `Foo`, just a misunderstanding on my part. – Vossler Jun 20 '18 at 11:35
  • Would this answer be a more proper C++-way than the answer by @Jarod42 ? This is partly an opinion-based question, true, but since it only influences which answer I mark as accepted (which is purely based on my opinion anyway), I don't see a problem with discussing that in the comments. – Vossler Jun 20 '18 at 11:38
  • IMHO, of course, this is the appropriate way: avoid unnecessary copy/move, initialize all members in the initialization list. A completely different issue is that of stack/heap, i.e. whether to use a `unique_ptr<>`, but that's another question, as the OP didn't hold a pointer to `Foo`, but an instance. So, Jarrod's answer fails in case of non-movable `Foo`. – Walter Jun 20 '18 at 11:43