0

This is a follow up to this question so if you need to see the Register class please refer to that question. Now based on the supplied answer I have written a function to do just that. I have 2 versions of the function one that will store the results back into the original and one that will return a copy. Here are my functions:

template<std::uint64_t N>
void reverseBitOrder( Register<N>& reg ) {
    auto str = reg.register_.to_string();
    std::reverse(str.begin(), str.end());
    auto x = vpc::Byte(str);
    reg.register_ = x;
}

// Extra unused parameter to avoid ambiguity
template<std::uint64_t N>
Register<N> reverseBitOrder(Register<N>& reg, bool _x_ /*not used*/ ) {
    auto str = reg.register_.to_string();
    std::reverse(str.begin(), str.end());
    auto x = vpc::Byte(str);
    Register<N> temp;
    temp.register_ = x;
    return temp;
}

The first one saves the value, the second returns a copy. My question is on the 2nd function I ended up adding a second parameter that is not used in order to avoid ambiguity due to overload resolution as functions can not be resolved on return types alone. So when I call this function I would have to pass either 0, 1, true or false to the function which has no effect.

Overall this in itself is not a very big deal, however, it doesn't seem very clean and concise to me. Are there any other ways to achieve this? I prefer not to make this a function of the class. My Register class or struct is complete as is and any kind of operations done on a register will be done by functions that take a reference to one or more register objects.

Francis Cugler
  • 7,788
  • 2
  • 28
  • 59
  • Do you really need two functions for this? Why not just use the function that returns a copy for both purposes? You can just assign the return value to the original variable if required. – P.W May 06 '19 at 06:06
  • @P.W True, but I was trying to keep the name of the function the same as it does the same thing the only difference; I did want two different functions for each purpose, one to overwrite the value and one to preserve and return a copy of the new value. – Francis Cugler May 06 '19 at 06:16
  • @P.W I guess I could have the bool parameter and default it to false and do an overwrite but if true is passed for copy then branch to another section, – Francis Cugler May 06 '19 at 06:17

2 Answers2

1

You can use std::optional to achieve this.

The return type of function template reverseBitOrder should be std::optional<vpc::Register<N>>.

The function template should be modified to:

template<std::uint64_t N>
std::optional<vpc::Register<N>> reverseBitOrder(vpc::Register<N>& reg, bool _x_ = false) {

    auto str = reg.register_.to_string();
    std::reverse(str.begin(), str.end());
    vpc::Register<N> temp;    

    if(_x_) //return copy
    {
        temp.register_ = vpc::Byte(str); //since register_ is a vpc::Byte. Generalize accordingly.
        return temp;
    }
    else //save in the same variable
    {
        reg.register_ = vpc::Byte(str);        
        return {};
    }
}

Live Demo here.

But you don't really need to use std::optional, since there is really no "failure" case in the function template.

P.W
  • 26,289
  • 6
  • 39
  • 76
  • I tried your implementation above but I was forced to have to use `auto` when returning a copied value. I'd rather store it into an instance of another Register. So I opted to do what you did here but without `std::optional`. My return type is just a `Register`. Works like a charm! I had thought about branching the code in a single function... but was also leaning toward separate functions... – Francis Cugler May 06 '19 at 07:09
  • This works good for `Register<8>` which is default size, but when I try to use it for larger registers, it is failing because of `reg.register_ = Byte(str)` ... – Francis Cugler May 06 '19 at 07:20
  • @FrancisCugler: Please see the comment after the assignment statement in the `if` clause. You have defined `register_ ` as `vpc::Byte` in the `Register` structure. You will have to generalize it. – P.W May 06 '19 at 07:23
0

If I understand correctly, both functions compute x and store it in a Register<N>, but one returns said object by value while the other stores the result in the function's argument reg.

Technically, this can be done by overloading on constness by defining those two functions:

template<std::uint64_t N> void reverseBitOrder( Register<N>& reg );
template<std::uint64_t N> Register<N> reverseBitOrder( Register<N> const& reg );

While this technically answers your question, that would be terrible design. If I'm not mistaken, the real issue is that what you want is this:

// Behaviour 1: value stored in reg
reverseBitOrder(reg);

// Behaviour 2: value stored in val, reg left untouched
auto val = reverseBitOrder(reg);

The problem with this is that whether the returned value is used or not is not something you can detect from inside the function.


The proper way to have one function do two things here would be to have a function with this signature:

template<std::uint64_t N> void reverseBitOrder( Register<N> const& inputReg, Register<N>& outputReg );

That function would use inputReg to compute x then store the result in outputReg, meaning you would use it like this:

// Behaviour 1: value stored in reg
reverseBitOrder(reg, reg);

// Behaviour 2: value stored in val, reg leftuntouched
reverseBitOrder(reg, val);

Now, if that really doesn't do it for you, there is a way to get the syntaxic sugar you're looking for, at the cost of unneeded complexity and of adding a constructor to Register<N>. It would look roughly like this:

// Forward declaration
template<std::uint64_t N>
class Register;

// Proxy class
template<std::uint64_t N>
struct RegisterProxy
{
    Register<N>* reg;
    TypeOfX x;

    ~RegisterProxy()
    {
        if (reg)
        {
            reg->register = x;
        }
    }
};

// Modified Register class
template<std::uint64_t N>
class Register
{
    ...

    Register(RegisterProxy& proxy)
    {
        ...
        register = proxy.x;
        proxy.reg = nullptr;
        ...
    }

    ...

    // Define the matching assignment operator too

    ...
};

// Your function
template<std::uint64_t N>
RegisterProxy<N> reverseBitOrder( Register<N>& reg )
{
    auto str = reg.register_.to_string();
    std::reverse(str.begin(), str.end());
    auto x = vpc::Byte(str);
    return RegisterProxy{reg, x};
}

That allows you to do

// Behaviour 1: temporary object immediately destroyed and value stored in reg
reverseBitOrder(reg);

// Behaviour 2: constructor called, value stored in val
//              the temporary object's destructor doesn't do anything, reg left untouched
auto val = reverseBitOrder(reg);

I can't recommend doing that though, it's more trouble than it's worth. I suggest using the solution I called "the proper way". It the least complicated and the one which is the hardest to use wrong later

Eternal
  • 2,648
  • 2
  • 15
  • 21
  • I appreciate the effort and there is some good advice there, however I'm trying to keep Register trivially copyable; so I need for its default constructor, copy constructor, etc. to all be default. – Francis Cugler May 06 '19 at 14:07
  • Yeah, as I said, using proxy and all is more trouble than it's worth. The proper way to do what you want to do is the one with inputReg and outputReg – Eternal May 06 '19 at 14:24
  • I modified my answer a bit to make it clearer what I actually recommend doing – Eternal May 06 '19 at 14:35