1

When restructuring some code I came across a 'problem' when returning a struct with 2 values. Now these really should be named for the documented effect. Later on I wanted to use tie so i changed the struct into inheriting from std::pair and just setting references. Now this actually works fine, but you will notice now my struct has the size of 24 as opposed to just 8 compared to the pair. 

#include <tuple>


struct Transaction : public std::pair<int, int> {
    using pair::pair;

  int& deducted = first;
  int& transfered = second;
};
//static_assert(sizeof(Transaction) == sizeof(std::pair<int, int>));//commenting in this line will fail compilation

Transaction makeTheTransaction();

void test(int& deduct, int& transfer) {
    std::tie(deduct, transfer) = makeTheTransaction(); 
}

The maybe obvious method is to change into member functions, however that is also too much 'boilerplate' for this case (then it just becomes easier not use tie later on). A direct memcpy is to eg. a tuple is straigt forward UB. A direct structured binding is also not doable since the variables is already in use.

My question is what is a better or minimal code solution disregarding reusable parts (and given that the size shouldn't grow beyond the size of 2 ints) ?

UPDATE: For this case I ended up just doing a plain struct and hold return in a temporary. For other users coming here, there is a library proposal to boost that seems to be able to convert any struct to tuple: https://github.com/apolukhin/magic_get/

darune
  • 10,480
  • 2
  • 24
  • 62
  • 2
    Simply: `struct Transaction {int deducted; int transfered; };` ? – Jarod42 Nov 20 '18 at 19:26
  • @Jarod42 that would fine, but it would fail to document what `makeTheTransaction()` returns. – darune Nov 20 '18 at 19:26
  • Why do you have to inherit from std::pair at all? You could just have makeTransaction return a plain-old pair, seeing as you don't appear to use the "deducted" and "transferred" fields (which wouldn't work as written, the references would need to be set in a constructor). – jwimberley Nov 20 '18 at 19:38
  • 1
    Inheriting std::pair is UB. – felix Nov 20 '18 at 19:40
  • I don't understand why you need to inherit from `std::pair` in order to use `std::tie`. Just use it: `auto t = std::tie(transaction.deducted, transaction.transferred)`. – Peter Ruderman Nov 20 '18 at 19:41
  • I didnt like inheriting pair to begin with. But like I said its for documenting the returns of the function. – darune Nov 20 '18 at 20:08

3 Answers3

1

It seems like you're over-complicating the problem to me. If you need to use std::tie, you can just use it. There's no need to alter your structure:

struct Transaction
{
  int deducted;
  int transferred;
};

// later...

auto t = std::tie(transaction.deducted, transaction.transferred);

If this is a pattern you use frequently, then you can wrap it in a little helper method:

struct Transaction
{
  int deducted;
  int transferred;

  auto to_tuple() const
  {
    return std::tie(deducted, transferred);
  }
};

You can also use this to assign to multiple variables at once, although I strongly discourage that. It's error prone and leads to brittle code. (For example, if you reverse the order of deduct and transfer in the example below, you've got a bug, but the compiler will give no warning or error.)

void test(int& deduct, int& transfer)
{
  std::tie(deduct, transfer) = makeTheTransaction().to_tuple();
}

Edit: On second thought...

If the goal here is just easy decomposition of the struct into variables, you could do that directly and avoid using pairs or tuples:

struct Transaction
{
  int deducted;
  int transferred;

  void decompose(int* deducted_, int* transferred_)
  {
    *deducted_ = deducted;
    *transferred_ = transferred;
  }
};

void test(int& deduct, int& transfer)
{
  makeTheTransaction().decompose(&deduct, &transfer);
}

This is still brittle, but at least now you'll get intellisense when you write the call to the decompose method, which will make the pattern a little less error prone.

Peter Ruderman
  • 12,241
  • 1
  • 36
  • 58
  • Your first suggestion with plain struct is also 'painfull'. While maybe not brittle as you say, it leads to overly complicated code. The thing is, first I have to create and name a temporary variable to hold the returned struct. Then I have to assign to the other 2 variables - now 3 lines of code. Then, in order to not polute the rest of the scope with the temporary, i have to put a block scope around those 3 lines. – darune Nov 21 '18 at 13:07
  • 1
    Well, I'd suggest two things. First, typing is not the bottleneck. Saving yourself a few lines a typing is rarely worth opening up the possibility of future bugs. We should strive for code that is correct by construction. Second, passing the entire struct around seems like the easier option. Why is it necessary to decompose the struct into multiple variables? – Peter Ruderman Nov 21 '18 at 13:12
  • I aggree with the passing around is better done with the struct. However, I find, it's sometimes the case when dealing with existing (legacy) code and restructuring it that you only want to take one or a few steps at a time for different reasons (eg. one reason is not knowing the size of surrounding code that needs to be updated - another is not loosing your focus too much). I now ended up just doing the plain struct and returning into a temporary (for now). Thanks for the feedback. – darune Nov 21 '18 at 13:31
0

I would simply go with:

struct Transaction
{
    int deducted;
    int transfered;
};

With usage similar to:

Transaction makeTheTransaction() { return {4, 2}; }

int main()
{
    auto [deduct, transfer] = makeTheTransaction(); 
    std::cout << deduct << transfer << std::endl;
}

Demo

Jarod42
  • 203,559
  • 14
  • 181
  • 302
0

Adding a conversion function works:

#include <tuple>

struct Transaction {
    std::pair<int, int> data_;

    operator std::tuple<int &, int &> () {
        return std::tie(data_.first, data_.second);
    }
};
static_assert(sizeof(Transaction) == sizeof(std::pair<int, int>));

Transaction makeTheTransaction() {
    return Transaction();
}

void test(int& deduct, int& transfer) {
    std::tie(deduct, transfer) = makeTheTransaction();
}

I don't think that this will cause any lifetime issue when using with std::tie.

This Transaction doesn't work with structured binding with two identifiers. But you can make it supports "tuple-like" binding by specializing std::get and std::tuple_size for it.

felix
  • 2,213
  • 7
  • 16