3
\$\begingroup\$

Being a newcomer to C++, I decided to write a simple program that would accomplish a simple task.

I settled on writing a program that, given the vertex and point on a parabola, find the "stretch factor", or \$a\$ in the vertex-form equation \$y=a(x-h)^2+k\$.

This is my first go at working with pointers/references, and I expect this attempt to be rather feeble. The program works fine, however, I would love to know if I am employing any measures that are inefficient or that break good practice.

/* 
 * pwidth.cpp
 * Given the vertex and a point on a parabola, attempts to find the "stretch factor",
 * which is the `a` var in vertex form equation y=a(x-h)^2+k.
 */

#include <iostream>
#include <cmath>

using namespace std;

float pwidth(int *vx, int *vy, int *px, int *py) {
    return (*py - *vy) / pow(*px - *vx, 2);
}

void gcin(int &cstor, const string &message) {
    cout << message;
    cin >> cstor;

    while (cin.fail()) {
        cout << message;

        cin.clear();
        cin.ignore(256, '\n');

        cin >> cstor;
    }
}

int main() {
    int vx = 0, vy = 0, px = 0, py = 0;

    gcin(vx, "enter vertex X: ");
    gcin(vy, "enter vertex Y: ");
    gcin(px, "enter point X: ");
    gcin(py, "enter point Y: ");

    cout << "a = " << pwidth(&vx, &vy, &px, &py) << endl;

    return 0;
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

This is fairly straightforward. Good work on separating out the user input into its own function! Here are some things that could use improvement:

Don't Use Pointers for Values

In your pwidth() function, you aren't changing the value of any of the arguments. They should be passed by value since they are simple types. You could even make them const to clarify that the function does not change them. (Some would say that's overkill for plain-old-datatypes. It's up to you.)

In general, in C++, it's better to use references than pointers because they are safer. But in both cases, you usually use a reference or pointer either when the value will be changed by the function, or in cases where the data is too large to pass on the stack. (You can use a const in the case where the data is too large to pass on the stack but won't change in the function.) The cstor reference in gcin() is great!

Naming

I think your names could be improved. pwidth and gcin don't mean anything to me. If pwidth() is calculating the "stretch factor", then call it stretch_factor() or something along those lines. (Or if the p stands for "parabola" just name it parabola_width().) I think gcin() would be better named something like prompt_user(), or get_user_input(), or get_int_from_user().

Likewise with variable names. px, vx, etc. should be point_x, vertex_x, etc. It's much easier to read.

Avoid using namespace std

This Stack Overflow question lays out why using namespace std is a bad idea.

Optimization

In your pwidth() function, you're calling the pow() function. That's a really expensive function. For the main() that you've written, it won't make much difference, but if you ever need to use pwidth() in time-critical code, that could become a bottleneck. Since all you're doing is squaring a value, I'd rewrite it like this:

float pwidth(int *vx, int *vy, int *px, int *py) {
    float delta_x = (*px - *vx);
    return (*py - *vy) / (delta_x * delta_x);
}

Why is that the case? Doing it locally is a single machine instruction - a multiply. Calling the pow() function involves first jumping to the code for the function, then doing some math, then jumping back to your code to finish up. Some of that may be hidden by the pipelining, but the math of the pow() function is also more complicated than a single instruction. You can test it for yourself by writing a loop like this and timing it:

float x = 1.2345678;
float sum = 0.0;
for (int i = 0; i < 10000000; ++i)
{
    sum += pow(x, 2.0);
}

Then do it again like, this:

float x = 1.2345678;
float sum = 0.0;
for (int i = 0; i < 10000000; ++i)
{
    sum += x * x;
}

(Just be careful that your compiler doesn't optimize away any of the loop when testing!)

\$\endgroup\$
4
  • \$\begingroup\$ Thank you! This is very valuable feedback, and it is much appreciated - I will refer to it as I continue my journey in C++. Could you explain, or provide a reference to an article which explains, why manually squaring a number a * a is faster than pow(a, 2)? This seems like an interesting phenomenon, and I'd be quite interested in knowing exactly why it is. \$\endgroup\$
    – Joseph
    Commented Dec 14, 2017 at 7:57
  • 2
    \$\begingroup\$ @JosephA., try to search for implementations of pow. It needs to handle quite a lot of stuff. \$\endgroup\$ Commented Dec 14, 2017 at 9:14
  • \$\begingroup\$ Perhaps mention that double is the default floating-point type (analogous to int); float is analogous to short and should be used much the same way. \$\endgroup\$ Commented Dec 14, 2017 at 10:35
  • \$\begingroup\$ @JosephA. I've added some info about the optimizations. Hope that helps. \$\endgroup\$ Commented Dec 15, 2017 at 2:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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