3
\$\begingroup\$

I'm working through a practice problem and am wondering if there's a more efficient way of writing my code and/or if the syntax for my pointers looks correct.

The practice problem:

  • Given the absolute temperatures Tc of a cold reservoir and Th of a hot reservoir (in degrees Kelvin), the coefficients of performance (cp) of the refrigeration cycle and heat pump are given by:
    • Refrig: cp = Tc / (Th - Tc)
    • Heat: cp = Th / (Th - Tc)
  • Write a program that will call a function to prompt the user for the values of Tc and Th . It then calls a function that calculates the two coefficients of performance. Finally, it calls a function to print the results.

Here is what I have:

#include <stdio.h>

void temperature(float *, float *);
void performance(float,float,float *, float *);
void PrintResults(float,float,float,float);

int main()
{
    float Tc, //cold reservoir; user input
        Th, //hot reservoir; user input
        Rcp, //refrigeration cycle coefficient of performance
        Hcp; // Heat pump coefficient of performance

    temperature(&Tc,&Th); //Prompts user for cold & hot reservoir temperatures
    performance(Tc,Th,&Rcp,&Hcp); //Calculates the coefficients of performance
    PrintResults(Tc,Th,Rcp,Hcp); //Prints the coefficients of performance
    return 0;
}
void temperature(float *ptrTc, float *ptrTh)
{
    printf("Enter the absolute temperature for a cold reservoir (in Kelvin): ");
    scanf("%f",&*ptrTc);
    printf("Enter the absolute temperature for a hot reservoir (in Kelvin): ");
    scanf("%f",&*ptrTh);
}
void performance(float Tc, float Th, float *ptrRcp, float *ptrHcp)
{
    *ptrRcp = Tc/(Th-Tc);
    *ptrHcp = Th/(Th-Tc);
}
void PrintResults(float Tc, float Th, float Rcp, float Hcp)
{
    printf("\nGiven the absolute temperatures of %.2f K for the cold reservoir",Tc);
    printf(" and %.2f K for the hot reservoir,",Th);
    printf(" the coefficient of performance for the refrigeration cycle is %.2f K",Rcp);
    printf(" and the coefficient of performance for the heat pump is %.2f K\n",Hcp);
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Use verbs for function names. get_temp() and calc_temp_performance(). Make pointers const in performance(). E.g. void performance(float, float, float *const, float *const) since pointers aren't and shouldn't be modified, only the data they point to is. Couple other tweaks, but some things to think about for now. \$\endgroup\$ – ahogen Apr 22 at 7:20
  • 1
    \$\begingroup\$ @ahogen as far as possible, parameters should not be modified, be it pointers or floats. Therefore it would make sense to either make all parameters const or none of them. I prefer the "none of them" variant and let a linter check that parameter variables aren't modified. \$\endgroup\$ – Roland Illig Apr 22 at 7:26
  • 1
    \$\begingroup\$ Is the coefficient really measured in Kelvin? It is calculated by dividing Kelvin by Kelvin, so it should be a scalar. \$\endgroup\$ – Roland Illig Apr 22 at 7:27
  • 1
    \$\begingroup\$ @RolandIllig Sure, but when things are passed by value, not by reference, then making it const doesn't do anything in terms of global data protection. I'm from embedded land, so if I can use that variable and not add something else to the stack, that's a win. My rule has been to const all pass-by-refs unless they must be modified. Leave pass-by-value un-const if you might use them. \$\endgroup\$ – ahogen Apr 22 at 16:27
4
\$\begingroup\$

if the syntax for my pointers looks correct

The syntax for the pointers is correct in all the function parameters, however, the pointer syntax is incorrect or unnecessary in the two scanf statements. The two output variables passed into temperature(float *, float*) are already addresses of variables so in this case the proper code would be

    scanf("%f", ptrTc);
    scanf("%f", ptrTh);

You would only need & if this was in the main program and Tc and Th were being read directly.

    float Tc;
    scanf("%f", &Tc);

if there's a more efficient way of writing my code

All four of the variables are related, it might be better to use a struct to contain them. That way if the program needed to be modified to loop through data it would be easier to build an array of all the values. Using a struct would also decrease the number of parameters for each function to one.

Error Checking
A best practice is to check all input data as it comes in and report any errors, especially for user input. Users can make errors such as entering characters rather than numbers. The temperature(float *, float*) function could return an integer the indicated success or failure.

Separate and Initialize in Variable Declarations
There are 2 things that can be improved in the following code:

    float Tc, //cold reservoir; user input
        Th, //hot reservoir; user input
        Rcp, //refrigeration cycle coefficient of performance
        Hcp; // Heat pump coefficient of performance

This code is hard to maintain because it is hard to add a variable to the list.

Not initializing variables can lead to hard to debug problems later, this is especially true of the C programming language because it does not automatically initialize variables to a zero or null value as some other languages do. Of the two problems this is actually the more serious one, the other might be considered style.

An possible example of an improved declaration is:

    float Tc = 0.0;  //cold reservoir; user input
    float Th = 0.0;  //hot reservoir; user input
    float Rcp = 0.0; //refrigeration cycle coefficient of performance
    float Hcp = 0.0; // Heat pump coefficient of performance

Spacing, Both Vertical and Horizontal
Within expressions is it common to put spaces between operators and operands, this makes the code more readable.

Vertical separation of one line between functions might make the code more readable as well.

\$\endgroup\$
  • 1
    \$\begingroup\$ regarding; float Tc = 0.0; and similar statements: The 0.0 is a double value. Suggest: float Tc = 0.0f; Note the trailing f to make it a float rather than a double \$\endgroup\$ – user3629249 Apr 23 at 17:17
  • \$\begingroup\$ @user3629249 Actually I think the OP should change all the floats to doubles, but I already gave the OP enough to chew on based on their skill set. \$\endgroup\$ – pacmaninbw Apr 23 at 19:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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