HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Time calculator for a given speed and file size

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filesizegiventimecalculatorforandspeed

Problem

I wrote this little code in c++. It calculates the needed time for a given speed of a medium (for example the speed is 1024 B/s, the file size is 1MB, it'll take 17 minutes and 4 seconds to finish). The code works, but I'm not sure if it's proper.

Can you tell me if it's okay or not?

#include 

int main()
{
    int transmission_speed; //speed of the transmission in bytes per seconds
    int file_size_mb; //file's size in MBs

    std::cout > transmission_speed;
    std::cout > file_size_mb;

    int file_size_b = file_size_mb *1024*1024;

    int seconds_needed = file_size_b / transmission_speed;

    int days_needed = (seconds_needed / 3600) / 24;
    seconds_needed -= days_needed*86400;

    int hours_needed = seconds_needed / 3600;
    seconds_needed -= hours_needed*3600;

    std::cout << "Days needed: " << days_needed << std::endl;
    std::cout << "Hours needed: " << hours_needed << std::endl;
    std::cout << "Seconds needed: " << seconds_needed << std::endl;
    return(0);
}

Solution

It's a very simple little program, however, there's a few comments to make:

  • All of the code is in main. Of course, for a program of this size, that doesn't really matter too much, however, you should prefer to break things up into self-contained functions where possible. For example, there should be a separate function here to actually do the calculations.



  • Use the correct data type for what you need. Can time ever be negative here? The answer is no, so I'd prefer to use unsigned over int.



  • You should be somewhat careful about overflow. Any filesize over 2048MB (2GB) will overflow the size of an int (assuming a 4 byte int - in actuality, an int is only technically required to be at least 2 bytes). Using an unsigned will change this to 4GB, however, if you expect any filesizes larger than that, you should look at another datatype (std::uint64_t perhaps).



  • Magic numbers. It's not so bad when dealing with time, because it's generally fairly obvious here, but numbers like 86400 shouldn't be shown as-is. They should be a named constant, such as const unsigned seconds_in_day = 86400.



I'd suggest breaking this up into a main function and a required_time(unsigned transmission_rate, unsigned file_size) function.

Context

StackExchange Code Review Q#30506, answer score: 11

Revisions (0)

No revisions yet.