Thread: Pointer help

  1. #1
    Registered User
    Join Date
    Sep 2020
    Posts
    31

    Pointer help

    Hello, I am a self taught in C, and new at that. I think its a stretch to call myself a programmer.

    In the first snippet, I pass the variable seed_temp to the psat function and it works perfectly.

    Code:
    while (index < 101)
    {
      float seed_temp = 20;
      do {
        psat1 = calc_psat(eA, eB, eC, seed_temp, gamma_e, x1);
        psat2 = calc_psat(wA, wB, wC, seed_temp, gamma_w, x2);
        seed_temp = seed_temp + 0.0001;
      } while (psat1 + psat2 < 760);
      boil_pt[index] = seed_temp;
    }

    In the 2nd snippet I'm trying to pass previously calculated seed_temp rather than start at a value of 20 each pass through the do while loop. However,in the 2nd snippet using a pointer, the do while loop does not iterate to satisfy the "< 760" condition.

    Code:
    float seed_temp = 70;
    float *p_seed_temp;
    p_seed_temp = &seed_temp;
    
    while (index < 101)
    {
    do {
    psat1 = calc_psat(eA, eB, eC, &seed_temp, gamma_e, x1);
    psat2 = calc_psat(wA, wB, wC, &seed_temp, gamma_w, x2);
    *p_seed_temp += 0.0001;
    }
    while (psat1 + psat2 < 760);
    boil_pt[index] = seed_temp;
    I hope I've explained the problem clearly. I cut some code out for brevity including an obvious curly bracket. But both programs compile and run. There might be other errors but the do while loop not executing is the current one.

    Thank you for any insight.

    gave up trying to fix code tags
    Last edited by Buckeye Bing; 09-09-2020 at 01:43 PM. Reason: too many code tags

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    In your first code snippet, seed_temp is a float, and you pass seed_temp as an argument to calc_psat.

    In your second code snippet, seed_temp is still a float, but you pass &seed_temp as an argument to calc_psat, i.e., you pass a pointer to float instead of a float, whereas previously you passed a float. You didn't show the declaration of calc_psat, but it doesn't look like that parameter is a void* such that calc_psat is able to interpret the argument's type differently depending on say, the value of another argument. So, in all likelihood, that's the problem: you should pass seed_temp, not &seed_temp, as the argument.

    I don't quite understand why you need p_seed_temp at all though. You could use seed_temp all the way.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Sep 2020
    Posts
    31
    Thank you laserlight.
    Passing seed_temp by reference does work fine. But it takes a while to run through all the iterations

    The psat declaration is "float calc_psat (float, float, float , float *, float, float);"

    The program computes the boiling temperature of a mixture, along with other parameters to it.
    There is a calculation for each 1% (while index<101). The temperature ranges from 70 to 100.
    The next temperature in the series will be very close to the previous temp.

    So the second version attempts to modify the original value of seed_temp before it is passed again. Rather than starting at 70 again.

    ChemE folks will recognize this as Antoines Eq.

    Sorry if this is long winded. Do you see anything obvious why the do while loop does not iterate?
    It work fine in version1.

    Thanks again

    Whomever fixed my code window- Thank you. I know how it works now

    Edit: So sorry. Very new at this. Temp is properly incrementing. What's not happening is the psat functions are only executing once. Though I want them to execute until the sum = 760.

    My biggest problem is likely describing the problem
    Last edited by Buckeye Bing; 09-09-2020 at 06:07 PM. Reason: Bad Analysis

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Buckeye Bing
    The psat declaration is "float calc_psat (float, float, float , float *, float, float);"
    Then your first code snippet is wrong, despite your assertion that it "works perfectly". In fact, I would expect your first code snippet to either never run at all or run into an infinite loop: you loop while index < 101, but you never modify index.

    Quote Originally Posted by Buckeye Bing
    There is a calculation for each 1% (while index<101). The temperature ranges from 70 to 100.
    By 1% you mean 1 degree Celsius? I suggest using a for loop then, and making it clear that you're dealing with a temperature rather than an "index":
    Code:
    for (int temperature = 70; temperature <= 100; ++temperature)
    {
        // ...
    }
    Quote Originally Posted by Buckeye Bing
    Do you see anything obvious why the do while loop does not iterate?
    It work fine in version1.
    I see obvious reasons why version 1 doesn't work, but you say it works. So something isn't right with what you're posting here. My suspicion is that something has been "lost in translation", i.e., the code that you're actually compiling isn't the same as the code that you're posting here.

    Quote Originally Posted by Buckeye Bing
    What's not happening is the psat functions are only executing once. Though I want them to execute until the sum = 760.
    If they execute once, then your do while loop is working. They are in fact executing until the sum = 760. The problem is that the sum reaches 760 before you expect it to, but that's not the fault of the do while loop.

    Here's what I suggest: post the smallest and simplest compilable program that demonstrates the problem. That is, post a program that we can compile and run with your test input, and then observe your actual output and compare it with your expected output. Right now, it is difficult to say what is the problem: for all we know it lies in calc_psat, which you did not show.

    This will also answer questions like: why did you make the fourth parameter of calc_psat be a pointer? Does calc_psat modify seed_temp?
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Sep 2020
    Posts
    31

    Red face

    Thank you so much for your review and comments. Assistance through the use of ones time is an appreciated gift. To squander that by misstatement or error is discourteous at best. My apologies on that score.

    Version 1 does work.
    Pointer help-version1-jpg

    Allow me to try again with a little context.


    I'm calculating the boil point curve of a ethanol/water mix for each fraction of the mixture; e.g. 0% ethanol-100% water, 1% ethnol-99% water... 100% ethanol-0% water. 100 fractions each with a different boil point. Yes, temperature is degrees C.


    Each component, at a given temperature exerts a vapor pressure (psat) against the atmosphere. When the sum of the 2 pressures equal atmospheric pressure the mixture begins to boil. That is the physical definition of boil. At the moment I'm using standard atmospheric pressure (sea level) of 760 mmHg. That's the basis of the inner loop.

    The outer loop does this 100 times; once for each fraction of the mix.
    1) 0% ethanol - 100% water
    2) 1% ethanol - 99% water, etc...

    The pointer syntax in V2 is correct but the while condition in the inner loop is not proper. The condition needs to account for the sum of psats being greater than or less than 760 (atm_press). That's the logic flaw. I think it can be fixed with a IF statement.

    The only reason to use a pointer for temperature is to avoid incrementing 10,000 times per degree over a range of 30 degrees.

    Thank you again.

    Edit: Thats a useless screenshot. Sorry
    Last edited by Buckeye Bing; 09-10-2020 at 02:38 PM.

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Buckeye Bing
    Version 1 does work.
    Yes, I believe you when you say it does work. What I am telling you is that according to the code that you posted in post #1, and the declaration of calc_psat that you provided in post #3, version 1 cannot possibly work: the outer loop index is not modified as the loop iterates (hence either the loop never runs or is an infinite loop) and the call to calc_psat is wrong because it passes a float where a float* is expected (a type error). This doesn't necessarily mean that version 1 doesn't work for you; it could mean that the code that you posted is inconsistent with the code that you compiled and ran. This makes it very difficult if not impossible to reason about your code.

    Quote Originally Posted by Buckeye Bing
    The outer loop does this 100 times; once for each fraction of the mix.
    1) 0% ethanol - 100% water
    2) 1% ethanol - 99% water, etc...
    Ah, so the outer loop isn't looping over a temperature, it loops over the percentage of ethanol? In that case the outer loop should look like this:
    Code:
    for (int ethanol_percentage = 0; ethanol_percentage <= 100; ++ethanol_percentage)
    {
        // ...
    }
    Quote Originally Posted by Buckeye Bing
    The condition needs to account for the sum of psats being greater than or less than 760 (atm_press). That's the logic flaw. I think it can be fixed with a IF statement.
    The condition says: loop while the sum of psats is less than 760, i.e., when the sum of psats is greater than or equal to 760, stop looping. Is that what you want to do?

    As for whether this can be fixed with an if statement: possibly, but why would you introduce an if statement when you just need to change the loop condition?

    My feeling is that your loop condition is correct to begin with: firstly, you seem to have done the same thing in version 1, and then it sounds consistent with "sum of the 2 pressures equal atmospheric pressure the mixture begins to boil" (unless you're saying that if sum of the 2 pressures is greater than atmospheric pressure the mixture does not boil).

    Quote Originally Posted by Buckeye Bing
    The only reason to use a pointer for temperature is to avoid incrementing 10,000 times per degree over a range of 30 degrees.
    Does this mean that calc_psat sets seed_temp for the next call of calc_psat? More simply: does calc_psat modify seed_temp?

    Furthermore, if that is your intention, then why do you still have *p_seed_temp += 0.0001 in version 2? The difference this time is that you don't reset seed_temp to 70 (or 20) on each outer loop iteration.

    EDIT:
    Quote Originally Posted by Buckeye Bing
    0% ethanol-100% water, 1% ethnol-99% water... 100% ethanol-0% water. 100 fractions each with a different boil point.
    That's actually 101 different percentages (or equivalently, fractions), not 100.
    Last edited by laserlight; 09-10-2020 at 04:34 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Sep 2020
    Posts
    31
    Quote Originally Posted by laserlight View Post
    it could mean that the code that you posted is inconsistent with the code that you compiled and ran. This makes it very difficult if not impossible to reason about your code.
    True. For brevity, I only showed the section of code I thought pertinent to the perceived problem. Further, I updated V1 with some features from V2 that do work. Btw, the V1 psat declaration does declare a float argument. It's the V2 that uses a pointer type.
    I'm creating confusion by limiting your view. And I'm trying to get my head around a FOR outer loop the way my program is currently constructed.

    I'm going to include the V1 code in entirety to clarify my objectives, namely: (I hope this isn't objectionable)
    Calculate boil point (liquid) and dew point (vapor) curve data using standard equations for vapor-liquid equilibria.
    Display Alcohol by Volume (ABV) of the vapor.
    Display ABV remaining in boiler.
    Export boil point and vapor ABV to csv file


    By convention:
    X1 is the liquid phase of ethanol
    X2 is the liquid phase of water
    Y1 is the vapor phase of ethanol
    Y2 is the vapor phase of water

    I've commented out the intended display and printf only data that 'truth out' the psat calcs.
    Also commented out the export to file code. It doesn't print all the data. I thought that is a separate thread.
    One last note; ethanol and water don't play well together chemically. That's addressed by the gamma function. And the conversion from molar fraction to ABV is cumbersome. But they work.

    If its OK I'll post V2 in a later post to compare how I'm trying be more efficient.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <math.h>
    
    
    /*/////////////////////////////////////////////////////////*/
    // File     Desktop\Still4
    // Author   Don Bing
    // Date   8-10-2020
    /*////////////////////////////////////////////////////////*/
    
    
    float calc_psat (float, float, float, float, float, float);
    float calc_gamma_e(float, float, float, float);
    float calc_gamma_w(float, float, float, float);
    float calc_abv(float);
    float calc_abw(float, float, float, float);
    
    
    /*Antoine Coefficients (C)*/
    float eA = 8.13484;
    float eB = 1662.48;
    float eC = 238.131;
    
    
    float wA = 8.05573;
    float wB = 1723.64;
    float wC = 233.076;
    
    
    /*Van Laar activity model constants*/
    float A12 = 1.68811;
    float A21 = 0.95268;
    
    
    float density_w = 0.998;    /*Density of water at 20C */
    float density_e = 0.785;
    float mm_w = 18.0153;
    float mm_e = 46.0684;
    
    
    /*******************************************************/
    
    
    int main()
    {
    int index = 0;
    int a = 0;
    float boil_pt[100];
    for ( a = 0; a < 101; a++ )
         boil_pt[a] = 0;
    
    
    float boiler_abv[100];
    for ( a = 0; a < 101; a++ )
         boiler_abv[a] = 0;
    
    
    float vapor_abv[100];
    for ( a = 0; a < 101; a++ )
         vapor_abv[a] = 0;
    
    
    float x1 = 0.0;
    float x2 = 1-x1;
    float y1 = 0;
    float y2 = 0;
    float psat1 = 0;
    float psat2 = 0;
    float boiler_abw = 0;
    float vapor_abw = 0;
    
    
    while (index < 101)
    {
    float seed_temp = 70;
    float atm_press = 760;
    
    
    float gamma_e = calc_gamma_e(x1, x2, A12, A21);
    float gamma_w = calc_gamma_w(x1, x2, A12, A21);
    
    
    do {
    psat1 = calc_psat(eA, eB, eC, seed_temp, gamma_e, x1);
    psat2 = calc_psat(wA, wB, wC, seed_temp, gamma_w, x2);
    seed_temp = seed_temp + 0.0001;
    }
    while (psat1 + psat2 < 760);
    
    
    boil_pt[index] = seed_temp;
    
    
    y1 = psat1 / atm_press;
    y2 = psat2 / atm_press;
    boiler_abw = calc_abw(mm_e, mm_w, x1, x2);
    vapor_abw = calc_abw(mm_e, mm_w, y1, y2);
    
    
    printf("\npercent       \tboil pt         \tpressure        \tpsat1        \tpsat2\n");
    printf("%1.2f           \t%2.5f           \t%3.4f           \t%3.2f        \t%3.2f\n",
            x1,             boil_pt[index],   psat1+psat2,      psat1,         psat2);
    
    
    
    
    boiler_abv[index] = calc_abv(boiler_abw);
    vapor_abv[index] = calc_abv(vapor_abw);
    
    
    
    
    /*printf("\nboil pt   \tabv         \tx1     \tx2     \tgamma_e   \tgamma_w       \tpressure     \ty1     \ty2\n");
    printf("%2.2f\t     \t%0.2f       \t%1.2f   \t%1.2f  \t%1.4f     \t%1.4f        \t%3.2f        \t%1.2f  \t%1.2f",
            boil_pt,    vapor_abv,     x1,      x2,      gamma_e,    gamma_w,       psat1+psat2,    y1,      y2);
    */
    x1 = x1 + 0.01;
    x2 = 1-x1;
    y1 = y1 + 0.01;
    y2 = 1 - y1;
    index++;
    }
    getch(0);
    
    
     /* FILE *pWrite;
    pWrite = fopen("vle.dat", "w");
    if (pWrite == NULL)
       printf("\nFile not opened\n");
    else
    {
    fprintf(pWrite,   "temp\t    vapor abv\t    boiler abv\n");
    fprintf(pWrite,  "%2.2f\t    %0.2f\t       %0.2f", boil_pt[index], vapor_abv[index], boiler_abv[index]);
    }
    */
    
    
    return (0);
    }
    /**********************FUNCTIONS*****************************/
    
    
    float calc_psat(float A, float B, float C, float st, float gamma, float x)
    {
    float psat = x*gamma*pow(10, (A-((B)/(C + st))));
    return psat;
    }
    /***************************************************/
    float calc_gamma_e(float x1, float x2, float a12, float a21)
    {
    float result = exp((a12 * pow(x2, 2)) / (pow(((a12 * x1 / a21) + x2),  2)));
    return (result);
    }
    /***************************************************/
    float calc_gamma_w(float x1, float x2, float a12, float a21)
    {
    float result = exp((a21 * pow(x1, 2)) /  (pow(((a21 * x2 / a12) + x1), 2)));
    return (result);
    }
    /***************************************************/
    float calc_abw(float mm_e, float mm_w, float mf1, float mf2)
    {
    float result = 0;
    result = mf1/(mf1 + mm_w/mm_e * mf2);
    
    
    return (result);
    }
    
    
    /***************************************************/
    
    
    float calc_abv(float abw)
    {
    float abv = 0;
    
    
    float a = -0.000039705486746795932;
    float b =  1.2709666849144778;
    float c = -0.40926819348115739;
    float d =  2.0463351302912738;
    float f = -7.8964816507513707;
    float g =  15.009692673927390;
    float h = -15.765836469736477;
    float i = 8.8142267038252680;
    float j = -2.0695760421183493;
    
    
    float temp = j;
    temp = temp * abw +i;
    temp = temp * abw +h;
    temp = temp * abw +g;
    temp = temp * abw +f;
    temp = temp * abw +d;
    temp = temp * abw +c;
    temp = temp * abw +b;
    abv = temp * abw +a;
    
    
    return (abv);
    
    }

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    This is legal syntax, but it doesn't really help a human being who looks at it:
    Code:
    float calc_psat (float, float, float, float, float, float);
    float calc_gamma_e(float, float, float, float);
    float calc_gamma_w(float, float, float, float);
    float calc_abv(float);
    float calc_abw(float, float, float, float);
    You should leave the parameter names in the function declarations so that someone looking at the function declaration has an idea what the parameters are for.

    It is good to have named constants rather than magic numbers, but in theory you could accidentally modify them, resulting in a bug that might be hard to find:
    Code:
    /*Antoine Coefficients (C)*/
    float eA = 8.13484;
    float eB = 1662.48;
    float eC = 238.131;
    
    
    float wA = 8.05573;
    float wB = 1723.64;
    float wC = 233.076;
    
    
    /*Van Laar activity model constants*/
    float A12 = 1.68811;
    float A21 = 0.95268;
    
    
    float density_w = 0.998;    /*Density of water at 20C */
    float density_e = 0.785;
    float mm_w = 18.0153;
    float mm_e = 46.0684;
    To avoid this, they should be declared const, or changed into macro constants.

    This is an off-by-one error:
    Code:
    float boil_pt[100];
    for ( a = 0; a < 101; a++ )
         boil_pt[a] = 0;
    On the last iteration of the loop, you assign 0 to boil_pt[100], but as there are only 100 float elements in boil_pt, boil_pt[100] does not exist. It's related to my observation that when you wrote that "0% ethanol-100% water, 1% ethnol-99% water... 100% ethanol-0% water. 100 fractions each with a different boil point", you made an off-by-one error: there are 101 fractions, not 100 fractions. To solve this, I would first declare a macro constant before the main function, e.g.
    Code:
    #define NUM_MIXTURE_PERCENTAGES 101
    Then I would use it:
    Code:
    float boil_pt[NUM_MIXTURE_PERCENTAGES];
    for ( a = 0; a < NUM_MIXTURE_PERCENTAGES; a++ )
         boil_pt[a] = 0;
    Having said that, because this is a special case where you want to set all the elements of the array of the initial value of 0, you don't actually need a loop:
    Code:
    float boil_pt[NUM_MIXTURE_PERCENTAGES] = {0};
    This way, you initialise the first element to 0, then the remaining elements are initialised to 0 by default. This only works for 0 though: if you tried it with 1, you would initialise the first element to 1, then the remaining elements would still be initialised to 0 by default.

    Anyway, the same goes for boiler_abv and vapor_abv.

    Quote Originally Posted by Buckeye Bing
    And I'm trying to get my head around a FOR outer loop the way my program is currently constructed.
    Let's take a look at your current loop with proper indentation:
    Code:
    int index = 0;
    
    /* ...
       lots of code/declarations in between obscuring what the value of index is just before the loop
       ... */
    
    while (index < 101)
    {
        float seed_temp = 70;
        float atm_press = 760;
    
        float gamma_e = calc_gamma_e(x1, x2, A12, A21);
        float gamma_w = calc_gamma_w(x1, x2, A12, A21);
    
        do {
            psat1 = calc_psat(eA, eB, eC, seed_temp, gamma_e, x1);
            psat2 = calc_psat(wA, wB, wC, seed_temp, gamma_w, x2);
            seed_temp = seed_temp + 0.0001;
        }
        while (psat1 + psat2 < 760);
    
        boil_pt[index] = seed_temp;
    
        y1 = psat1 / atm_press;
        y2 = psat2 / atm_press;
        boiler_abw = calc_abw(mm_e, mm_w, x1, x2);
        vapor_abw = calc_abw(mm_e, mm_w, y1, y2);
    
        printf("\npercent       \tboil pt         \tpressure        \tpsat1        \tpsat2\n");
        printf("%1.2f           \t%2.5f           \t%3.4f           \t%3.2f        \t%3.2f\n",
                x1,             boil_pt[index],   psat1+psat2,      psat1,         psat2);
    
        boiler_abv[index] = calc_abv(boiler_abw);
        vapor_abv[index] = calc_abv(vapor_abw);
    
        x1 = x1 + 0.01;
        x2 = 1-x1;
        y1 = y1 + 0.01;
        y2 = 1 - y1;
        index++;
    }
    So, it turns out that you are modifying index in the loop after all: right at the end. It would hence be clearer if you wrote this:
    Code:
    /* ...
       lots of code/declarations
       ... */
    
    for (int index = 0; index < NUM_MIXTURE_PERCENTAGES; index++)
    {
        float seed_temp = 70;
        float atm_press = 760;
    
        float gamma_e = calc_gamma_e(x1, x2, A12, A21);
        float gamma_w = calc_gamma_w(x1, x2, A12, A21);
    
        do {
            psat1 = calc_psat(eA, eB, eC, seed_temp, gamma_e, x1);
            psat2 = calc_psat(wA, wB, wC, seed_temp, gamma_w, x2);
            seed_temp = seed_temp + 0.0001;
        }
        while (psat1 + psat2 < 760);
    
        boil_pt[index] = seed_temp;
    
        y1 = psat1 / atm_press;
        y2 = psat2 / atm_press;
        boiler_abw = calc_abw(mm_e, mm_w, x1, x2);
        vapor_abw = calc_abw(mm_e, mm_w, y1, y2);
    
        printf("\npercent       \tboil pt         \tpressure        \tpsat1        \tpsat2\n");
        printf("%1.2f           \t%2.5f           \t%3.4f           \t%3.2f        \t%3.2f\n",
                x1,             boil_pt[index],   psat1+psat2,      psat1,         psat2);
    
        boiler_abv[index] = calc_abv(boiler_abw);
        vapor_abv[index] = calc_abv(vapor_abw);
    
        x1 = x1 + 0.01;
        x2 = 1-x1;
        y1 = y1 + 0.01;
        y2 = 1 - y1;
    }
    Are these constants a to j well known names in the literature?
    Code:
    float calc_abv(float abw)
    {
    float abv = 0;
     
     
    float a = -0.000039705486746795932;
    float b =  1.2709666849144778;
    float c = -0.40926819348115739;
    float d =  2.0463351302912738;
    float f = -7.8964816507513707;
    float g =  15.009692673927390;
    float h = -15.765836469736477;
    float i = 8.8142267038252680;
    float j = -2.0695760421183493;
     
     
    float temp = j;
    temp = temp * abw +i;
    temp = temp * abw +h;
    temp = temp * abw +g;
    temp = temp * abw +f;
    temp = temp * abw +d;
    temp = temp * abw +c;
    temp = temp * abw +b;
    abv = temp * abw +a;
     
     
    return (abv);
     
    }
    If they are, keeping them is okay, though you might want to place a comment with some explanation. If they are not, then you're just naming constants for the sake of naming them without providing descriptive names, so there's no point. I would prefer to use a static array and a loop instead:
    Code:
    float calc_abv(float abw)
    {
        static const float modifiers[] = {
            -2.0695760421183493,
            8.8142267038252680,
            -15.765836469736477,
            15.009692673927390,
            -7.8964816507513707,
            2.0463351302912738,
            -0.40926819348115739,
            1.2709666849144778,
            -0.000039705486746795932
        };
    
        float abv = 0.0f;
        for (size_t i = 0; i < sizeof(modifiers) / sizeof(modifiers[0]); ++i)
        {
            abv = abv * abw + modifiers[i];
        }
        return abv;
    }
    Then again, even if the constants a to j are well known names in the literature, you might use this array+loop approach anyway and just write the domain-specific constant names as comments next to the array elements.
    Last edited by laserlight; 09-10-2020 at 08:04 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Registered User
    Join Date
    Sep 2020
    Posts
    31
    Wow!

    Just simply wow. I wouldn't have expected such a thorough and constructive review of my code. Besides improving my program your suggestions have expanded my knowledge of good programming practice. For that, I am grateful and absolutely delighted.

    Sincerest Thanks and Warm Regards.

    Don

  10. #10
    Registered User
    Join Date
    Sep 2020
    Posts
    31
    Quote Originally Posted by laserlight View Post

    Are these constants a to j well known names in the literature?
    Code:
    float calc_abv(float abw)
    {
    float abv = 0;
     
     
    float a = -0.000039705486746795932;
    float b =  1.2709666849144778;
    float c = -0.40926819348115739;
    float d =  2.0463351302912738;
    float f = -7.8964816507513707;
    float g =  15.009692673927390;
    float h = -15.765836469736477;
    float i = 8.8142267038252680;
    float j = -2.0695760421183493;
     
     
    float temp = j;
    temp = temp * abw +i;
    temp = temp * abw +h;
    temp = temp * abw +g;
    temp = temp * abw +f;
    temp = temp * abw +d;
    temp = temp * abw +c;
    temp = temp * abw +b;
    abv = temp * abw +a;
     
     
    return (abv);
     
    }
    If they are, keeping them is okay, though you might want to place a comment with some explanation. If they are not, then you're just naming constants for the sake of naming them without providing descriptive names, so there's no point. I would prefer to use a static array and a loop instead:
    Code:
    float calc_abv(float abw)
    {
        static const float modifiers[] = {
            -2.0695760421183493,
            8.8142267038252680,
            -15.765836469736477,
            15.009692673927390,
            -7.8964816507513707,
            2.0463351302912738,
            -0.40926819348115739,
            1.2709666849144778,
            -0.000039705486746795932
        };
    
        float abv = 0.0f;
        for (size_t i = 0; i < sizeof(modifiers) / sizeof(modifiers[0]); ++i)
        {
            abv = abv * abw + modifiers[i];
        }
        return abv;
    }
    Then again, even if the constants a to j are well known names in the literature, you might use this array+loop approach anyway and just write the domain-specific constant names as comments next to the array elements.
    Not constants, per se; but coefficients to an empirical model. Valid only at 20 deg C. I would have preferred to use the original octic polynomial shown in the image.
    Pointer help-abw2abv-jpg

    But without an exponent operator "^" it seemed overly cumbersome. I'm correct, aren't I, that C does not have such an operator? The format of the code block was for readability to the uninitiated

    The provided array+loop approach is almost understandable to me. I don't understand the 'f' in this initialization float abv = 0.0f;

    Thanks.

  11. #11
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Quote Originally Posted by Buckeye Bing View Post
    Not constants, per se; but coefficients to an empirical model. Valid only at 20 deg C. I would have preferred to use the original octic polynomial shown in the image.
    As well as being valid only for 20C it is also only valid for inputs between 0.0 and 1.0 inclusive. Both of these constraints should be added as a comment. For the 0.0 to 1.0 you should probably add that as a check in your function as well.

    Additionally, for this kind of thing double will give more accurate results than float (i.e. if it was me I'd change all your float keywords to double)

    Quote Originally Posted by Buckeye Bing View Post
    I don't understand the 'f' in this initialization float abv = 0.0f;
    The 'f' suffix says that the constant, 0.0 in this case, is a float. Without it the constant 0.0 is double.
    Last edited by Hodor; 09-11-2020 at 07:41 PM.

  12. #12
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    I'd double check your implementation of calc_abv()... it doesn't seem to match your picture

  13. #13
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Buckeye Bing
    Not constants, per se; but coefficients to an empirical model.
    Coefficients would imply that they are used to multiply, but you're adding them. As Hodor pointed out, you aren't implementing the equation correctly. You can still use the array+loop method, but you'll need to adjust what you add and what you multiply. You may find that you also need another variable to keep track of the multiples of x (i.e., your abw is the x, but you're not always multiplying by x; you're multiplying by multiples of x).

    Quote Originally Posted by Buckeye Bing
    But without an exponent operator "^" it seemed overly cumbersome. I'm correct, aren't I, that C does not have such an operator?
    You're right that C does not have an exponent operator, but the C standard library does have a few functions for computing numbers raised by an exponent. In fact, you've used two of them yourself to implement calc_psat and calc_gamma_e: pow and exp respectively.

    In this case though, you don't want to use such a function because you can compute the multiples of x as the loop iterates.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  14. #14
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Quote Originally Posted by laserlight View Post
    As Hodor pointed out, you aren't implementing the equation correctly.
    Looking at it again I was mistaken. I didn't notice that the "coefficients" were being added backwards and the implementation is using methods described here: Horner's method - Wikipedia

    Apologies for not noticing that earlier

  15. #15
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Quote Originally Posted by Hodor View Post
    Looking at it again I was mistaken. I didn't notice that the "coefficients" were being added backwards and the implementation is using methods described here: Horner's method - Wikipedia
    I don't know which of the below implementations below I prefer, but I do know that comments should be added (if there was comments in the initial code I'd not have initially thought it was implemented incorrectly, for example )

    Code:
    #include <stdio.h>
    #include <math.h>
    
    double calc_abv(double abw);
    double calc_abv2(double abw);
    double calc_abv3(double abw);
    
    int main()
    {
        for (double abw = 0.0; abw <= 1.0; abw += 0.1) {
            printf("%lf, %lf, %lf, %lf\n", abw, calc_abv(abw), calc_abv2(abw), calc_abv3(abw));
        }
        return 0;
    }
    
    
    double calc_abv(double abw)
    {
        const double a = -0.000039705486746795932;
        const double b =  1.2709666849144778;
        const double c = -0.40926819348115739;
        const double d =  2.0463351302912738;
        const double f = -7.8964816507513707;
        const double g =  15.009692673927390;
        const double h = -15.765836469736477;
        const double i = 8.8142267038252680;
        const double j = -2.0695760421183493;
    
    
        double temp = j;
        temp = temp * abw +i;
        temp = temp * abw +h;
        temp = temp * abw +g;
        temp = temp * abw +f;
        temp = temp * abw +d;
        temp = temp * abw +c;
        temp = temp * abw +b;
        double abv = temp * abw +a;
    
        return abv;
    }
    
    double calc_abv2(double abw)
    {
        const double a = -0.000039705486746795932;
        const double b =  1.2709666849144778;
        const double c = -0.40926819348115739;
        const double d =  2.0463351302912738;
        const double f = -7.8964816507513707;
        const double g =  15.009692673927390;
        const double h = -15.765836469736477;
        const double i = 8.8142267038252680;
        const double j = -2.0695760421183493;
    
        return  a +
                b * abw +
                c * pow(abw, 2) +
                d * pow(abw, 3) +
                f * pow(abw, 4) +
                g * pow(abw, 5) +
                h * pow(abw, 6) +
                i * pow(abw, 7) +
                j * pow(abw, 8)
        ;
    }
    
    double calc_abv3(double abw)
    {
        static const double coefficients[] = {
            -0.000039705486746795932,
            1.2709666849144778,
            -0.40926819348115739,
            2.0463351302912738,
            -7.8964816507513707,
            15.009692673927390,
            -15.765836469736477,
            8.8142267038252680,
            -2.0695760421183493
        };
    
        double abv = coefficients[0];
        double abw_mult = abw;
        for (size_t i = 1; i < sizeof(coefficients) / sizeof(coefficients[0]); ++i)
        {
            abv = abv + coefficients[i] * abw_mult;
            abw_mult *= abw;
        }
        return abv;
    }
    Edit: or laserlight's implementation, of course
    Last edited by Hodor; 09-11-2020 at 11:49 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 1
    Last Post: 08-05-2018, 12:14 AM
  2. Replies: 4
    Last Post: 08-18-2015, 03:13 PM
  3. Replies: 8
    Last Post: 03-01-2015, 12:39 AM
  4. Replies: 3
    Last Post: 10-30-2009, 04:41 PM
  5. Replies: 4
    Last Post: 08-27-2007, 11:51 PM

Tags for this Thread