current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I have the following two vectors in C++11:

vector<unsigned char> left = { 1, 2, 3, 4, 5, 6 }; // it is meant to represent the number: 123456
vector<unsigned char> right = { 1, 2, 3, 4 }; // meant to be number: 1234

After a function processes them, they become:

left = { 6, 5, 4, 3, 2, 1 };
right = { 4, 3, 2, 1 };

Now, I need to complete the shorter one with zeros at the end until they are equal in size(). I have thefollowing method:

void bigint::process_operands( vector<unsigned char> & left, vector<unsigned char> & right )
{
    size_t left_size = left.size();
    size_t right_size = right.size();

    if( left_size < right_size )
    {
        size_t size_diff = right_size - left_size;

        for( size_t i = 0; i != size_diff; ++i )
            left.push_back( '0' );
    } else if( right_size < left_size )
    {
        size_t size_diff = left_size - right_size;

        for( size_t i = 0; i != size_diff; ++i )
            right.push_back( '0' );
    }
}

Is there any better implementation?

share|improve this question
add comment

3 Answers

up vote 4 down vote accepted

How about something like this:

auto sz = std::max(left.size(), right.size());
left.resize(sz, '0');
right.resize(sz, '0');

.resize() will resize the container to the given size, and fill the new elements (if any) with the given value.

share|improve this answer
 
I am not sure abut its performance, but it compiles... –  Victor 16 hours ago
1  
@Victor What are you unsure about? It's at least as efficient as (probably more than) using back_inserter or push_back. If you're worried about max, replace it with a conditional (although it's almost guaranteed to get inlined). –  DBRalir 5 hours ago
add comment
up vote 9 down vote

Firstly, given that you're using vector without namespace qualification (that is, not as std::vector), I assume you have a using namespace std; in this code. This is generally a bad idea, but especially so in this case. You use vector<unsigned char> & right (and vector<unsigned char> left), however, there is a std::right (and a std::left) defined in <ios>. This could potentially lead to some very confusing compiler errors.

Using a few tricks from <algorithm>, we can shorten the above code considerably.

void bigint::process_operands(vector<unsigned char> & left_, vector<unsigned char> & right_)
{
   auto& shorter = (left_.size() < right_.size()) ? left_ : right_;
   std::fill_n(std::back_inserter(shorter), std::abs(left_.size() - right_.size()), '0');
}

This is much more terse, but I find that using the standard library actually makes this code easier to read and more clear as to exactly what it is doing.

share|improve this answer
add comment
up vote 4 down vote

I’d just like to point out a very short version of your own code. It is certainly not as C++ish as a vector::insert or fill_n solution, but what it does is pretty obvious.

void bigint::process_operands( vector<unsigned char> & left, vector<unsigned char> & right )
{
    while (left.size() < right.size())
        left.push_back( '0' );
    while (right.size() < left.size())
        right.push_back( '0' );
}
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.