13

In my project I have some functions like

std::tuple<VAO, Mesh, ShaderProgram> LoadWavefront(std::string filename);

That I can use like this:

VAO teapotVAO;
Mesh teapotMesh;
ShaderProgram teapotShader;
std::tie(teapotVAO, teapotMesh, teapotShader)
    = LoadWavefront("assets/teapot.obj");

The problem is, this requires each of those classes to have a default constructor that creates them in an invalid state, which is error prone. How do I get around that without having to std::get<> each item? Is there an elegant way to do this?

Dan
  • 12,409
  • 3
  • 50
  • 87
  • I think you mean "to have a default constructor that creates them in *a valid state*"? As for `std::get<>`, do you want to avoid that altogether, or would something that minimises the number of uses also be acceptable? –  Nov 21 '14 at 19:03
  • 7
    My suggestion: Don't use a `tuple`, but a custom type. It seems you're not programming generically, so there's no reason to use a type which does not specify decent names for its members. – dyp Nov 21 '14 at 19:09
  • @hvd Unfortunately there are sometimes no reasonable default constructors for these types. – Dan Nov 21 '14 at 21:16
  • @dyp That actually makes a lot of sense. Then instead of a factory function I just have a constructor, instead of `teapotMesh` I have `teapot.Mesh`, and all of the nice cleanup/locality of reference comes for free. – Dan Nov 21 '14 at 21:30

4 Answers4

17

There is an inverted-control flow style that could be useful.

LoadWavefront("assets/teapot.obj", [&]( VAO teapotVAO, Mesh teapotMesh, ShaderProgram teapotShader ){
  // code
});

with VAO& reference-style instead optional. In this case, the return value of the lambda could be used as the return value of the LoadWavefront, with a default lambda that just forwards all 3 arguments out allowing "old style" access if you want. If you only want one, or want to do some stuff after it is loaded, you can also do that.

Now, LoadWavefront should probably return a future as it is an IO function. In this case, a future of tuple. We can make the above pattern a bit more generic:

template<class... Ts, class F>
auto unpack( std::tuple<Ts...>&& tup, F&& f ); // magic

and do

unpack( LoadWavefront("assets/teapot.obj"), [&]( VAO teapotVAO, Mesh teapotMesh, ShaderProgram teapotShader ){
  // code
});

unpack can also be taught about std::futures and automatically create a future of the result.

This can lead to some annoying levels of brackets. We could steal a page from functional programming if we want to be insane:

LoadWavefront("assets/teapot.obj")
*sync_next* [&]( VAO teapotVAO, Mesh teapotMesh, ShaderProgram teapotShader ){
  // code
};

where LoadWavefront returns a std::future<std::tuple>. The named operator *sync_next* takes a std::future on the left hand side and a lambda on the right hand side, negotiates a calling convention (first trying to flatten tuples), and continues the future as a deferred call. (note that on windows, the std::future that async returns fails to .wait() on destruction, in violation of the standard).

This is, however, an insane approach. There may be more code like this coming down the type with the proposed await, but it will provide much cleaner syntax to handle it.


Anyhow, here is a complete implementation of an infix *then* named operator, just because live example

#include <utility>
#include <tuple>
#include <iostream>
#include <future>

// a better std::result_of:
template<class Sig,class=void>
struct invoke_result {};
template<class F, class... Args>
struct invoke_result<F(Args...), decltype(void(std::declval<F>()(std::declval<Args>()...)))>
{
  using type = decltype(std::declval<F>()(std::declval<Args>()...));
};
template<class Sig>
using invoke_t = typename invoke_result<Sig>::type;

// complete named operator library in about a dozen lines of code:
namespace named_operator {
  template<class D>struct make_operator{};

  template<class T, class O> struct half_apply { T&& lhs; };

  template<class Lhs, class Op>
  half_apply<Lhs, Op> operator*( Lhs&& lhs, make_operator<Op> ) {
      return {std::forward<Lhs>(lhs)};
  }

  template<class Lhs, class Op, class Rhs>
  auto operator*( half_apply<Lhs, Op>&& lhs, Rhs&& rhs )
  -> decltype( invoke( std::forward<Lhs>(lhs.lhs), Op{}, std::forward<Rhs>(rhs) ) )
  {
      return invoke( std::forward<Lhs>(lhs.lhs), Op{}, std::forward<Rhs>(rhs) );
  }
}

// create a named operator then:
static struct then_t:named_operator::make_operator<then_t> {} then;

namespace details {
  template<size_t...Is, class Tup, class F>
  auto invoke_helper( std::index_sequence<Is...>, Tup&& tup, F&& f )
  -> invoke_t<F(typename std::tuple_element<Is,Tup>::type...)>
  {
      return std::forward<F>(f)( std::get<Is>(std::forward<Tup>(tup))... );
  }
}

// first overload of A *then* B handles tuple and tuple-like return values:
template<class Tup, class F>
auto invoke( Tup&& tup, then_t, F&& f )
-> decltype( details::invoke_helper( std::make_index_sequence< std::tuple_size<std::decay_t<Tup>>{} >{}, std::forward<Tup>(tup), std::forward<F>(f) ) )
{
  return details::invoke_helper( std::make_index_sequence< std::tuple_size<std::decay_t<Tup>>{} >{}, std::forward<Tup>(tup), std::forward<F>(f) );
}

// second overload of A *then* B
// only applies if above does not:
template<class T, class F>
auto invoke( T&& t, then_t, F&& f, ... )
-> invoke_t< F(T) >
{
  return std::forward<F>(f)(std::forward<T>(t));
}
// support for std::future *then* lambda, optional really.
// note it is defined recursively, so a std::future< std::tuple >
// will auto-unpack into a multi-argument lambda:
template<class X, class F>
auto invoke( std::future<X> x, then_t, F&& f )
-> std::future< decltype( std::move(x).get() *then* std::declval<F>() ) >
{
  return std::async( std::launch::async,
    [x = std::move(x), f = std::forward<F>(f)]() mutable {
      return std::move(x).get() *then* std::move(f);
    }
  );
}

int main()
{
  7
  *then* [](int x){ std::cout << x << "\n"; };

  std::make_tuple( 3, 2 )
  *then* [](int x, int y){ std::cout << x << "," << y << "\n"; };

  std::future<void> later =
    std::async( std::launch::async, []{ return 42; } )
    *then* [](int x){ return x/2; }
    *then* [](int x){ std::cout << x << "\n"; };
  later.wait();
}

this will let you do the following:

LoadWaveFront("assets/teapot.obj")
*then* [&]( VAO teapotVAO, Mesh teapotMesh, ShaderProgram teapotShader ){
  // code
}

which I find cute.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • 1
    interesting solution to an old problem! – IdeaHat Nov 21 '14 at 19:34
  • Agreed, that functional technique insane :) – Justin R. Nov 21 '14 at 19:34
  • I really like the idea of using a `future`, it lends itself naturally to putting (possibly a queue of) blocking disk operations in another thread. – Dan Nov 21 '14 at 21:34
  • @Dan See the `std::future` `then` proposals. Having a chain of lambdas, where each produces what the next consumes, that you can then bundle up and put into an async call and extract the `std::future`, and you can start with a `std::future` that produces what a lambda consumes and get the resulting "composed" `std::future` out, are all useful. These things should be coming down the pipe in C++ -- go look at the standardization for inspiration on how to do it yourself. – Yakk - Adam Nevraumont Nov 21 '14 at 21:41
  • 1
    I never thought I'd see someone using CPS in C++. That's clever. There's no real need to bake `std::future` into `LoadWaveFront`, though; the call can always be wrapped. – Stuart Olsen Nov 21 '14 at 21:55
  • @StuartOlsen One advantage of the `std::future` is to point out to the user that you should basically never use it synchronously. But yes, method-chaining via a `*then*`, and wrapping the entire chain up in an `async` may be better. I stole the continuation passing style pattern from `template struct synchronize { template auto read( F&& f );` "make any object thread safe" pattern, when you need to do something both before and after the task. Here, there may be nothing that needs doing after the task, but the pattern is similar. – Yakk - Adam Nevraumont Nov 21 '14 at 22:01
  • @Yakk At that point, it sounds like what you're really looking for is Haskell. :) But yes, there are definitely advantages to doing it that way, though it seems somehow more idiomatic to me to compose this sort of code from synchronous building blocks and control-flow primitives like `async` and `then` than to make the latter part of interface of the former. – Stuart Olsen Nov 21 '14 at 22:24
  • @StuartOlsen Naw, I'm prejudiced against mandatory garbage collectors. So instead I write code that imports Haskell into C++. See `*then*` above, you know it by a different name I suspect. – Yakk - Adam Nevraumont Nov 21 '14 at 22:26
  • Isn't this `then` synchronous? It reminds me more of an infix data flow specifier than the proposed `future::then`. – dyp Nov 21 '14 at 22:31
  • @dyp yes it is. You can overload `std::future` on the left and `invokeable` on the right to return `std::future>` easily (roughly as many characters as this comment really). This is one reason why I *really* like my 12-line named operator library, it makes that kind of thing easy. – Yakk - Adam Nevraumont Nov 22 '14 at 01:20
  • 1
    @dyp I was off by one. `std::future *then*` support required 306 characters, the above comment was 307. Serves me right for guessing. – Yakk - Adam Nevraumont Nov 22 '14 at 01:35
4

You could use boost::optional:

boost::optional<VAO> teapotVAO;
boost::optional<Mesh> teapotMesh;
boost::optional<ShaderProgram> teapotShader;
std::tie(teapotVAO, teapotMesh, teapotShader)
    = LoadWavefront("assets/teapot.obj");

Of course you'd have to change the way you access these values to always do *teapotVAO, but at least the compiler will let you know if you mess up any of the access.

Barry
  • 286,269
  • 29
  • 621
  • 977
4

Lets go even further, and assume there is no default constructor for those classes.

One option is something like this:

auto tup = LoadWavefront("assets/teapot.obj");
VAO teapotVAO(std::move(std::get<0>(tup)));
Mesh teapotMesh(std::move(std::get<1>(tup)));
ShaderProgram teapotShader(std::move(std::get<2>(tup)));

This still leaves around the tup as a mostly cleaned up opject, which is less than ideal.

But wait...why does those even need to have ownership?

auto tup = LoadWavefront("assets/teapot.obj");
VAO& teapotVAO=std::get<0>(tup);
Mesh& teapotMesh=std::get<1>(tup);
ShaderProgram& teapotShader=std::get<2>(tup);

As long as the references are in the same scope as the returned tuple, there is no problem here.

Personally, this seems like a clear place where one should use a set of smart pointers instead of this nonsense:

LoadWavefront(const char*,std::unique_ptr<VAO>&,std::unique_ptr<Mesh>&,std::unique_ptr<ShaderProgram>&);

std::unique_ptr<VAO> teapotVAO;
std::unique_ptr<Mesh> teapotMesh;
std::unique_ptr<ShaderProgram> teapotShader;
LoadWavefront("assets/teapot.obj",teapotVAO,teapotMesh,teapotShader);

This will take care of the ownership issue and allow a sensible null state.

Edit: /u/dyp pointed that you could use the following with the original output style

std::unique_ptr<VAO> teapotVAO;
std::unique_ptr<Mesh> teapotMesh;
std::unique_ptr<ShaderProgram> teapotShader;
std::tie(teapotVAO,teapotMesh,teapotShader) = LoadWavefront("assets/teapot.obj");
IdeaHat
  • 7,641
  • 1
  • 22
  • 53
  • 2
    Instead of using out-parameters, you could use the OP's style with `unique_ptr`s and return them in a tuple. – dyp Nov 21 '14 at 19:10
  • @dyp oh yeah rhr will allow that just fine. WIll edit – IdeaHat Nov 21 '14 at 19:12
  • @dyp hrm...any knowledge on if this could be RVO? – IdeaHat Nov 21 '14 at 19:15
  • 1
    We cannot see if RVO is happening in this code, since there is no `return` ;) The (presumably) temporary created as a return value of `LoadWavefront` cannot be elided, since this line does not construct an object from it (it merely assigns to a tuple). But we don't need RVO here, all this line does is to copy three pointers and zero the sources afterwards. – dyp Nov 21 '14 at 19:18
  • @dyp completely agree that its not needed here...unique_ptr wouldn't let it happen if it wasn't a move, so the worst case is you'll have 6 pointer copies (3 to return, 3 to value). RVO would be more useful in the case of the original problem (maybe?) but who knows. – IdeaHat Nov 21 '14 at 19:22
  • I like this answer, but the downside to the `unique_ptr` solution is that they can be null, which is almost as bad as having invalidly constructed objects. – Dan Nov 21 '14 at 21:21
  • @Dan I disagree. NULL pointers are well known constructs for capturing non-initialized types, but even more so. std::unique_ptr has a default cast to bool, so you can easily do `if (teapotVAO)` and feel good about it. Some stl implementations will even give you intelligent exceptions on this stuff in debug mode, but regardless it is guaranteed to be either NULL or pointing to an object. – IdeaHat Nov 21 '14 at 22:11
  • @Dan further reading on the subject which came to no conclusive answer is http://stackoverflow.com/questions/14360706/when-to-use-boostoptional-and-when-to-use-stdunique-ptr-in-cases-when-you-wa, I think both sides have merit, so this is probably a case of "To each their own" – IdeaHat Nov 21 '14 at 22:15
4

How do I get around that without having to std::get<> each item? Is there an elegant way to do this?

Return by value, instead of returning by "values" (which is what this std::tuple allows you to do).

API changes:

class Wavefront
{
public:
    Wavefront(VAO v, Mesh m, ShaderProgram sp); // use whatever construction
                                                // suits you here; you will
                                                // only use it internally
                                                // in the load function, anyway
    const VAO& vao() const;
    const Mesh& mesh() const;
    const ShaderProgram& shader() const;
};

Wavefront LoadWavefront(std::string filename);
utnapistim
  • 26,809
  • 3
  • 46
  • 82