3

So I have this great number mapping function:

double map(double input, double input_start, double input_end, double output_start, double output_end)
    {
        /* Note, "slope" below is a constant for given numbers, so if you are calculating
        a lot of output values, it makes sense to calculate it once.  It also makes
        understanding the code easier */

        double slope = (output_end - output_start) / (input_end - input_start);
        return output_start + slope * (input - input_start);
    }

Courtesy of Alok Singhal: Mapping a numeric range onto another

And he is correct, I call the function many many times, but usually with the same input and output start/end arguments.

What do you think is the cleanest/simplest solution to having slope calculated once, I can think of many ideas (such as just making a Map class and have slope calculated in the constructor) But I am new to programming, and usually my ideas are not the best! The most ultimate, probably would be that the function itself knows if slope needs to be recalculated, but that might get messy quickly. I dunno!

aName
  • 257
  • 2
  • 8
  • 2
    Looking at this code snippet, I would assume that any kind of caching would be much more expensive than calculating it. How about making a slope class? If a value changes, you can reset a bool that triggers new calculation? – JVApen Jun 12 '19 at 09:10
  • But may as well just put slope inside a Map class, right? makes everything easier to manage. BTW, yikes! I just realised I should have put this question on Codereview.StackExchange! – aName Jun 12 '19 at 09:14
  • Do you know when inputs change? (current computation is easy, adding check (or caching) would probably reduce performance). – Jarod42 Jun 12 '19 at 09:14
  • std::map (or any alike) are much more complex to do a lookup, so no – JVApen Jun 12 '19 at 09:15
  • @Jarod42 Yeah. I'm really beginning to see the advantage of a Map object with slope calculator function (and/or constructor) – aName Jun 12 '19 at 09:16
  • Btw, have you profiled your code and identify it as bottleneck? – Jarod42 Jun 12 '19 at 09:25
  • No just thinking ahead, and trying to apply good programming principles at all times. – aName Jun 12 '19 at 09:26
  • 2
    > I call the function many many times, but usually with the same input and output start/end arguments Well, the first step is the critically examine this statement. Do you *really* call it many times? How many times? Haw many do you think is *many*? And if you do, so what? Computers are fast. Are the many calls actually *too expensive*? Next, examine *why* you call the function many times with the same values. You do not provide any context for us to decide whether there is a flaw in the calling code that causes an excessive number of calls. – Raedwald Jun 12 '19 at 09:36
  • @aName Good programming technique would indicate that you group your concepts correctly instead of passing 4 related arguments. Once you have those correct, the rest will follow. Don't blindly optimize without understanding what the costs are. [Compiler explorer](godbolt.org) is a nice tool to show you the generated assembly of snippets. – JVApen Jun 12 '19 at 09:49
  • Could you explain what you mean by this "Good programming technique would indicate that you group your concepts correctly instead of passing 4 related arguments." - Do you mean like passing in a single struct? Perhaps you could point me to some further reading? Thank you – aName Jun 12 '19 at 10:05

1 Answers1

1

As already hinted in the comments, using caching with an advanced structure won't do you much good. Calculating 2 minimus operations and a divide is really hard to beat. And most likely not a performance problem.

My recommendation: compile your code with -O3 to let the optimizer do its job and profile where you are loosing your time.

Looking at your actual problem, you have 2 steps, a constant part and a variable part.

To deal with this kind of caching, classes are perfect. You create them at the root of your data and propagate them onwards. A function it could make use of the internals.

For example:

class Slope {
    double input_start{};
    double output_start{};
    double slope{};

public:
    Slope(double input_start, double input_end, double output_start, double output_end)
        : input_start{input_start}
        , output_start{output_start}
    {
    slope = (output_end - output_start) / (input_end - input_start);
    }

  double map(double input) const
        return output_start + slope * (input - input_start);
    }

When data changes, you create a new instance that can then be used (but not with new, but rather just Slope slope(0., 1., 40., 50.); auto res = slope.map(0.2);).

Example usage:

for (std::string s : file)
{
      double inStart = getInStart(s);
      ...;
      Slope slope = Slope{inStart, inEnd, outStart, outEnd};
      ...;
      for (double in : points)
      {
           double end = slope.map(in);
      }
}
Peter O.
  • 32,158
  • 14
  • 82
  • 96
JVApen
  • 11,008
  • 5
  • 31
  • 67
  • What do you mean "(please don't allocate memory for it)"? I'm going to create a Map class with a setRange function that calculates slope. but giving you credit for your help. Thank you – aName Jun 12 '19 at 09:34
  • 2
    @aName: Don't use `new`. just `Slope slope(0., 1., 40., 50.); auto res = slope.map(0.2);`. – Jarod42 Jun 12 '19 at 09:40
  • Like @Jarod42 suggested, I've added a usage to the question. – JVApen Jun 12 '19 at 09:42