patterncppModerate
Time calculator for a given speed and file size
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?
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:
I'd suggest breaking this up into a
- 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
unsignedoverint.
- 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, anintis only technically required to be at least 2 bytes). Using anunsignedwill change this to 4GB, however, if you expect any filesizes larger than that, you should look at another datatype (std::uint64_tperhaps).
- Magic numbers. It's not so bad when dealing with time, because it's generally fairly obvious here, but numbers like
86400shouldn't be shown as-is. They should be a named constant, such asconst 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.