2
\$\begingroup\$

I am new to coding and made myself my first little program, which lets me calculate the amount of money I make per hour doing a transcription project. While my code works (yay!) and I am proud of my first attempt at creating a program from scratch, I'm not sure I went about it the most efficient way. Could someone please look this over and let me know where I could improve?

#include <stdio.h>
#include <cs50.h>

int main(void)

{
    float clip_time;
    float real_time;
    float pay_rate; 
    float pay_amt;
    float hours; 

do{

printf("Transcription Rates\n");
printf("Clip Time: ");
clip_time = get_float();
}
while(clip_time <= 0);

printf("%.2f min clip pays: ", clip_time);
    pay_rate = get_float();

printf("So, $%.2f for the project \n", pay_rate * clip_time);
    pay_amt = pay_rate*clip_time;
    {

do{
    printf("Time to Complete Clip (in minutes): ");
    real_time = get_float();

   printf("Equals %.2f hours \n", real_time/60);
   hours = real_time/60;

}
while(real_time <= 0);

    {
        printf("Real time spent per audio minute: %.2f \n", real_time / clip_time);
}

{   
        printf("Hourly pay $%.2f div by %.2f hours equals $%.2f ph \n", pay_amt, hours, pay_amt / hours);
    }

    }       
}
\$\endgroup\$
3
\$\begingroup\$

I think you have written your code well! Here are some steps you can have in mind when coding:

  1. Always comment the code, even though it is self-explanatory, it might not be in 1-5 years later. Comment so that you and others trying to help you understand what your goal was.
  2. You can always improve your code, one way is follow DRY mindset, which is Don't Repeat Yourself. You can use functions to wrap your code, instead of using the same code in multiple places.
  3. Learn as you go, watch others how they organize your code, and implement their structure into your code.
\$\endgroup\$
2
\$\begingroup\$

While I understand that this question is based on the CS50 problem sets, it would be better to move beyond using the traditional tools cs50 offers. Some advice:

  • Consider using the scanf() function to get input from the user. The course may or may not (I've just started the course myself) introduce this function, so just as a heads up, here is a scanf() function to use for accepting all types of input from the user, instead of invoking several different functions.

  • Your code formatting is not at its best. You use far too many curly braces where they are not even required, like:

    {   
            printf("Hourly pay $%.2f div by %.2f hours equals $%.2f ph \n", pay_amt, hours, pay_amt / hours);
        }
    

    This statement doesn't need to be enclosed in curly braces

  • Another point, along the lines of formatting, is to use proper indentation. Indentation and appropriate use of whitespace plays a key role in enhancing the readability of your code.

  • An important note: it is good practice to use the return 0; statement at the end of your main() function. Even if the compiler will terminate the function with a zero-status if the program ran successfully, it would be a better habit to get into the habit of adding this statement because the main() function has a return value of type int.

  • You can also declare all your float variables in one line:

    float clip_time, real_time, pay_rate, pay_amt, hours;
    

    Instead of declaring them on separate lines. This is not a recommendation, but just a formatting tip that many people would use.

\$\endgroup\$
1
\$\begingroup\$
  • cs50 is not a standard header. So, it reduces the portability of your code.
  • You might as well as return 0; at the end of main as it is not implicit in C89 (ansi C)
  • This part:

     printf("Equals %.2f hours \n", real_time/60);
     hours = real_time/60;
    

    should be outside the do...while loop as it needs to be executed only if the user enters a valid input for real_time.

  • Using doubles are better than floats because they have much better precision.
  • You could inform the user that invalid input was typed when the user enters some invalid input. But I do not know what get_float returns on non-number inputs. If you do, you can check for it too.
  • The part to get input can be separated into another function to avoid repetition.
  • Last but not least, indent your code properly. Currently, it looks like a mess with a few extra braces lurking around here and there.

Putting it all together, we get

#include <stdio.h>
#include <cs50.h> /* Warning! Non-standard header! */

double get_double_input(const char* prompt)
{
    double input;

    for(;;)
    {
        printf("%s ", prompt);
        fflush(stdout);

        input = get_double();
        if(input > 0)
        {
            break;
        }

        fputs("Invalid input! Input should be greater than 0! Try again \n", stderr);
    }

    return input;
}

int main(void)
{
    double clip_time;
    double real_time;
    double pay_rate; 
    double pay_amt;
    double hours; 

    printf("Transcription Rates \n");
    clip_time = get_double_input("Clip Time: ");

    char prompt[50];
    sprintf(prompt, "%.2f min clip pays: ", clip_time);
    pay_rate = get_double_input(prompt);

    printf("So, $%.2f for the project \n", pay_rate * clip_time);
    pay_amt = pay_rate * clip_time;

    real_time = get_double_input("Time to Complete Clip (in minutes): ");

    printf("Equals %.2f hours \n", real_time/60);
    hours = real_time/60;

    printf("Real time spent per audio minute: %.2f \n", real_time / clip_time);
    printf("Hourly pay $%.2f div by %.2f hours equals $%.2f ph \n", pay_amt, hours, pay_amt / hours);

    return 0;
}

Untested Code ↑

\$\endgroup\$

Your Answer

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