CODEWITHSUNDEEP
c++
Vikas
Vikas

Reputation: 2260

Modifying const parameter temporarily

I have a function which conceptually supposed to not modify the parameter. The parameter object is a big object (a vector with 10000 or more), so I don't want to create a copy. What is a moral way of doing this thing in c++.

double computeDelta(const vector< double > &grid, unsigned int index, double newvalue) {
     // compute something  on old grid
     double oldvalue = grid[index]
     // change grid temporarily
     grid[index] = newvalue; // this is illegal because of const (How to do this is question)
     // compute something on new grid
     // restore original grid
     grid[index] = oldvalue
     return // difference of old thing and new thing
}

Upvotes: 2

Views: 137

Answers (6)

Useless
Useless

Reputation: 67743

1. pass by value

This is guaranteed perfectly safe, and is easy. Only do something more complex if profiling demonstrates you really need it

double computeDelta(vector<double> grid, unsigned int index, double newvalue) {
    double before = compute(grid);
    grid[index] = newvalue;
    double after = compute(grid);
    return after-before;
}

2. pass by non-const reference

This requires the caller to trust you, and they may have to make a copy anyway if there are multiple threads

// I modify grid in-place, but promise to revert it before exiting
double computeDelta(vector<double> &grid, unsigned int index, double newvalue) {
    double before = compute(grid);
    // we can do something much more elegant if C++11 lambdas are allowed
    struct swapper {
        double &value;
        double oldvalue;
        swapper(double &v, double newvalue) : value(v), oldvalue(v) {
            value = newvalue;
        }
        ~swapper() { value = oldvalue; }
    } guard(grid[index], newvalue);
    double after = compute(grid);
    return after-before;
}

3. interpose a read-through wrapper

This is the only safe (const-correct) way to take a const ref without forcing a copy. It requires that the computation is templated on the container type (or alternatively on the iterator type, and you proxy the iterator instead). Despite avoiding a copy, it may be slower depending on the access pattern

double computeDelta(vector<double> const &grid, unsigned int index, double newvalue) {
    double before = compute(grid);
    // assuming only operator[] is used by compute
    struct overlay {
        vector<double> const &base;
        unsigned index;
        double value;
        overlay(vector<double> const &b, unsigned i, double v)
         : base(b), index(i), value(v) {}
        double operator[] (vector<double>::size_type i) const {
            return (i == index) ? value : base[i];
        }
        vector<double>::size_type size() const { return base.size(); }
    };
    double after = compute(overlay(grid, index, newvalue));
    return after-before;
}

Upvotes: 3

djechlin
djechlin

Reputation: 60778

Firstly, don't modify the vector - in your simple example I don't see why you can't just work in a temporary variable.

But const_cast is arguably the right thing to do here, given that this is a situation in which you need to judiciously "cheat" on const-ness. Note that this will break under multithreading, as your client code may assume you are not modifying the vector but you are. It will also break if you throw exceptions midway through, i.e., if you do not carefully guarantee atomicity.

Safest thing to do - just remove the const declaration and explain how it is modified in comments, i.e., modified intermediately.

Upvotes: 2

Ari
Ari

Reputation: 1092

I don't think casting away the constness is a good idea. You could get rid of it, or if you want to keep the constness, write some adapter code to handle it for you. There are many ways you could do this. e.g.

As you iterate over the vector, if you get to index, then you return your special value.

for (int i = 0; i < grid.size(); ++i) {
    if(i == index) { /* do something with newvalue */ }
    else { /* do something with grid[i] */ }
}

Or you could write a wrapper class to do the same thing

class GridWrapper {
public:
    GridWrapper(const std::vector<double>& grid, unsigned int idx, double val) 
    : m_grid(grid), m_idx(idx), m_val(val) {}

    double& operator[](unsigned int pos) {
        if (pos == m_idx) return val;
        else return m_grid[pos];
    }
};

Or you could use something like boost::transform_iterator to do the same thing.

Upvotes: 0

Tom Tanner
Tom Tanner

Reputation: 9354

Remove the const and be very careful to clean up after yourself.

If you're taking a const reference parameter, you are guaranteeing not to change it. If you start changing it, firstly this may well break horribly in a multi threaded program and secondly, ot is also possible that the vector is in some read only space [note: I'm aware this is quite tricky with C++ technology at the moment, but I could see a C++11 compiler working out it could create an initialised vector in read only space] - in which case calling your function will crash. Really, it would be better if it did not compile.

Upvotes: 0

lip
lip

Reputation: 688

You can use const_cast:

double computeDelta(const vector< double > &grid, unsigned int index, double newvalue) {
     // compute something  on old grid
     double oldvalue = grid[index];
     // change grid temporarily
     const_cast<vector<double>&>(grid)[index] = newvalue;
     // restore original grid
     const_cast<vector<double>&>(grid)[index] = oldvalue;
     return // difference of old thing and new thing
}

Upvotes: 2

segfault
segfault

Reputation: 5939

Just remove the const modifier from the function declaration.

Upvotes: 2

Related Questions