Skip to content

Vector scale implementation is incorrect #60

@jherico

Description

@jherico

Scale(t_ValueToBeScaled, t_min, t_max, 0, 1); // !!!!!!!!!!!!!!!!??????????

The existing void Scale(vector<double>& a_Values, const double a_tr_min, const double a_tr_max) function ignores the passed a_tr_min and a_tr_max.

Delegating to the existing Scale(double&...) function is inefficient because it causes the intermediate range value to be recomputed every call.

The a_Values = t_ValuesScaled; is inefficient because it invokes a vector copy requiring linear time, whereas a_Values.swap(t_ValuesScaled); would be constant time as it only exchanges the pointers for each vector.

The t_ValuesScaled.push_back(t_ValueToBeScaled); is inefficient because it can potentially trigger vector reallocation inside the loop. The final size of t_ValuesScaled is known at the start, so there's no reason not to pre-allocate storage by using vector::reserve

Here is an alternative implementation:

void Scale(std::vector<double>& a_Values, const double a_tr_min, const double a_tr_max)
{
    if (a_Values.empty()) {
        return;
    }

    double t_max = std::numeric_limits<double>::min(), t_min = std::numeric_limits<double>::max();
    GetMaxMin(a_Values, t_min, t_max);
    auto range = t_max - t_min;
    auto destRange = a_tr_max - a_tr_min;
    auto scaleValue = destRange / range;
    std::vector<double> t_ValuesScaled;
    t_ValuesScaled.reserve(a_Values.size());
    for (auto t_ValueToBeScaled : a_Values) {
        t_ValueToBeScaled -= t_min;
        t_ValueToBeScaled *= scaleValue;
        t_ValueToBeScaled += a_tr_min;
        t_ValuesScaled.push_back(t_ValueToBeScaled);
    }
    a_Values.swap(t_ValuesScaled);
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions