Skip to main content
added 101 characters in body
Source Link
Yuushi
  • 11.1k
  • 2
  • 31
  • 66

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.

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. 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.

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.

Source Link
Yuushi
  • 11.1k
  • 2
  • 31
  • 66

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. 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.