Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

The code is basically fine.

The if condition is perhaps a little complicated though. To interpret it you have to read "if not ((i is not a multiple of 3) and (i is not a multiple of 5))", which is a double negative.

Given the problem statement, we'd expect something more like "if (i is a multiple of 3) or (i is a multiple of 5)", so:

if ((i % 3 == 0) || (i % 5 == 0))

Nitpicking:

  • If we don't need argc and argv, we can declare main as int main() with no arguments.
  • The return 0; at the end of main happens automatically in C++, so we don't have to write it ourselves.

Note that there exists an arithmetic solution to the problem, which is much faster (it could even be done at compile-time in C++).

We want to sum all the multiples of 3 or 5 less than 1000. We can think about the sequence of multiples of 3 as (3, 6, 9, 12, 15, ..., 999), which is the same as 3 * (1, 2, 3, 4, 5, ..., 333). [...]

Such a sequence, where the difference between each number is constant, is called a finite arithmetic progression [and the sum is] a finite arithmetic series. The formula for the sum is 1/2 * n * (a_1 + a_n). where n is the number of terms being added, a_1 is the first element in the sequence, and a_n is the last element in the sequence.

The code is basically fine.

The if condition is perhaps a little complicated though. To interpret it you have to read "if not ((i is not a multiple of 3) and (i is not a multiple of 5))", which is a double negative.

Given the problem statement, we'd expect something more like "if (i is a multiple of 3) or (i is a multiple of 5)", so:

if ((i % 3 == 0) || (i % 5 == 0))

Nitpicking:

  • If we don't need argc and argv, we can declare main as int main() with no arguments.
  • The return 0; at the end of main happens automatically in C++, so we don't have to write it ourselves.

Note that there exists an arithmetic solution to the problem, which is much faster (it could even be done at compile-time in C++).

We want to sum all the multiples of 3 or 5 less than 1000. We can think about the sequence of multiples of 3 as (3, 6, 9, 12, 15, ..., 999), which is the same as 3 * (1, 2, 3, 4, 5, ..., 333). [...]

Such a sequence, where the difference between each number is constant, is called a finite arithmetic progression [and the sum is] a finite arithmetic series. The formula for the sum is 1/2 * n * (a_1 + a_n). where n is the number of terms being added, a_1 is the first element in the sequence, and a_n is the last element in the sequence.

The code is basically fine.

The if condition is perhaps a little complicated though. To interpret it you have to read "if not ((i is not a multiple of 3) and (i is not a multiple of 5))", which is a double negative.

Given the problem statement, we'd expect something more like "if (i is a multiple of 3) or (i is a multiple of 5)", so:

if ((i % 3 == 0) || (i % 5 == 0))

Nitpicking:

  • If we don't need argc and argv, we can declare main as int main() with no arguments.
  • The return 0; at the end of main happens automatically in C++, so we don't have to write it ourselves.

Note that there exists an arithmetic solution to the problem, which is much faster (it could even be done at compile-time in C++).

We want to sum all the multiples of 3 or 5 less than 1000. We can think about the sequence of multiples of 3 as (3, 6, 9, 12, 15, ..., 999), which is the same as 3 * (1, 2, 3, 4, 5, ..., 333). [...]

Such a sequence, where the difference between each number is constant, is called a finite arithmetic progression [and the sum is] a finite arithmetic series. The formula for the sum is 1/2 * n * (a_1 + a_n). where n is the number of terms being added, a_1 is the first element in the sequence, and a_n is the last element in the sequence.

Source Link
user673679
  • 12.2k
  • 2
  • 34
  • 65

The code is basically fine.

The if condition is perhaps a little complicated though. To interpret it you have to read "if not ((i is not a multiple of 3) and (i is not a multiple of 5))", which is a double negative.

Given the problem statement, we'd expect something more like "if (i is a multiple of 3) or (i is a multiple of 5)", so:

if ((i % 3 == 0) || (i % 5 == 0))

Nitpicking:

  • If we don't need argc and argv, we can declare main as int main() with no arguments.
  • The return 0; at the end of main happens automatically in C++, so we don't have to write it ourselves.

Note that there exists an arithmetic solution to the problem, which is much faster (it could even be done at compile-time in C++).

We want to sum all the multiples of 3 or 5 less than 1000. We can think about the sequence of multiples of 3 as (3, 6, 9, 12, 15, ..., 999), which is the same as 3 * (1, 2, 3, 4, 5, ..., 333). [...]

Such a sequence, where the difference between each number is constant, is called a finite arithmetic progression [and the sum is] a finite arithmetic series. The formula for the sum is 1/2 * n * (a_1 + a_n). where n is the number of terms being added, a_1 is the first element in the sequence, and a_n is the last element in the sequence.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /