Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

I see some things that may help your improve your program.

Think about a better algorithm

At the moment, for each item in the list, the code looks for a matching number in the entire list. This is \${\bf O}(n^2)\$ complexity. First, the inner loop could at least start with the next element instead of starting from 0. That would save a little time and is valid because a matching number couldn't possibly be before the outer loop's index or it would already have been found. Second, consider how one might be able to solve the problem using a single pass through the data. You would need another local data structure for this approach, but it may still be better for long arrays. Finally, you might consider using two algorithms -- one for a short list and one for a long one.

Don't abuse using namespace std

Especially in a very simple program like this, there's little reason to use that line. Putting using namespace std at the top of every program is a bad habita bad habit that you'd do well to avoid.

Don't abuse the trinary operator

The code currently uses the :? operator to decide which string to display. Better would be to make that into a conventional if..else construct for better clarity.

Be wary of signed versus unsigned

On some machines and for some compilers (such as gcc and clang on a Linux machine), list.size() will return an unsigned number, but it's being compared to an int within each for loop. One way to deal with that would be this:

for (decltype(list.size()) i = 0; i < list.size(); i++)

A neater way to do that would be to make a specific variable type:

using size_type = decltype(list.size());

Omit unused variables

Because argc and argv are unused, you could use the alternative form of main:

int main ()

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main. Here are some observations that may help you improve your program.

I see some things that may help your improve your program.

Think about a better algorithm

At the moment, for each item in the list, the code looks for a matching number in the entire list. This is \${\bf O}(n^2)\$ complexity. First, the inner loop could at least start with the next element instead of starting from 0. That would save a little time and is valid because a matching number couldn't possibly be before the outer loop's index or it would already have been found. Second, consider how one might be able to solve the problem using a single pass through the data. You would need another local data structure for this approach, but it may still be better for long arrays. Finally, you might consider using two algorithms -- one for a short list and one for a long one.

Don't abuse using namespace std

Especially in a very simple program like this, there's little reason to use that line. Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Don't abuse the trinary operator

The code currently uses the :? operator to decide which string to display. Better would be to make that into a conventional if..else construct for better clarity.

Be wary of signed versus unsigned

On some machines and for some compilers (such as gcc and clang on a Linux machine), list.size() will return an unsigned number, but it's being compared to an int within each for loop. One way to deal with that would be this:

for (decltype(list.size()) i = 0; i < list.size(); i++)

A neater way to do that would be to make a specific variable type:

using size_type = decltype(list.size());

Omit unused variables

Because argc and argv are unused, you could use the alternative form of main:

int main ()

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main. Here are some observations that may help you improve your program.

I see some things that may help your improve your program.

Think about a better algorithm

At the moment, for each item in the list, the code looks for a matching number in the entire list. This is \${\bf O}(n^2)\$ complexity. First, the inner loop could at least start with the next element instead of starting from 0. That would save a little time and is valid because a matching number couldn't possibly be before the outer loop's index or it would already have been found. Second, consider how one might be able to solve the problem using a single pass through the data. You would need another local data structure for this approach, but it may still be better for long arrays. Finally, you might consider using two algorithms -- one for a short list and one for a long one.

Don't abuse using namespace std

Especially in a very simple program like this, there's little reason to use that line. Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Don't abuse the trinary operator

The code currently uses the :? operator to decide which string to display. Better would be to make that into a conventional if..else construct for better clarity.

Be wary of signed versus unsigned

On some machines and for some compilers (such as gcc and clang on a Linux machine), list.size() will return an unsigned number, but it's being compared to an int within each for loop. One way to deal with that would be this:

for (decltype(list.size()) i = 0; i < list.size(); i++)

A neater way to do that would be to make a specific variable type:

using size_type = decltype(list.size());

Omit unused variables

Because argc and argv are unused, you could use the alternative form of main:

int main ()

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main. Here are some observations that may help you improve your program.

Source Link
Edward
  • 66.6k
  • 4
  • 118
  • 282

I see some things that may help your improve your program.

Think about a better algorithm

At the moment, for each item in the list, the code looks for a matching number in the entire list. This is \${\bf O}(n^2)\$ complexity. First, the inner loop could at least start with the next element instead of starting from 0. That would save a little time and is valid because a matching number couldn't possibly be before the outer loop's index or it would already have been found. Second, consider how one might be able to solve the problem using a single pass through the data. You would need another local data structure for this approach, but it may still be better for long arrays. Finally, you might consider using two algorithms -- one for a short list and one for a long one.

Don't abuse using namespace std

Especially in a very simple program like this, there's little reason to use that line. Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Don't abuse the trinary operator

The code currently uses the :? operator to decide which string to display. Better would be to make that into a conventional if..else construct for better clarity.

Be wary of signed versus unsigned

On some machines and for some compilers (such as gcc and clang on a Linux machine), list.size() will return an unsigned number, but it's being compared to an int within each for loop. One way to deal with that would be this:

for (decltype(list.size()) i = 0; i < list.size(); i++)

A neater way to do that would be to make a specific variable type:

using size_type = decltype(list.size());

Omit unused variables

Because argc and argv are unused, you could use the alternative form of main:

int main ()

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main. Here are some observations that may help you improve your program.