Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
Community Bot
Community Bot
  • 1

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practicebad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know what compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know what compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know what compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

added 1 character in body
Source Link
Rakete1111
Rakete1111
  • 2.6k
  • 1
  • 15
  • 23

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know whywhat compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know why compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know what compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

deleted 1 character in body
Source Link
Rakete1111
Rakete1111
  • 2.6k
  • 1
  • 15
  • 23

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know why compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compilercompile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success).

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know why compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compiler on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success).

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

As others have already commented on the performance issues, I will comment on your C++ usage.

Don't use using namespace std;

It is bad practice to use using namespace std;, although this is that bad for small programs such as yours, it would be better if you could get rid of that habit as soon as possible :) (If you know what you are doing, then you may keep it - but only if you really know what you are doing).

What's _Vector_iterator?

What's _Vector_iterator? Wait, is that a compiler specific type to denote an iterator for std::vector? Not good! I don't know why compiler is in use, but using compiler specific types is very bad because:

  • You have no guarantee that _Vector_iterator is still going to be a vector iterator when the compiler is upgraded
  • Your code doesn't compile on any other compiler

This is not only for _Vector_iterator, but for every other type beginning with a _ that you are using.

Instead, use std::vector<T>::iterator for an iterator, std::vector<T>::value_type for T and so on. If you can use C++11, use auto:

auto it = std::find(voters.begin(), voters.end(), name);

Avoid std::endl

std::endl outputs a newline, and flushes the output stream. Depending on the platform, this is not always needed, and if you know you need to flush the stream to see any output, better be explicit and use std::flush.

Flushing is expensive, so this may slow down your program. Additionally, to speed up input/output, you can unsync with stdio (the sync is used if you are using std::printf and std::cout interchangeably, but if you are only using one of them, you can safely turn in off). This will give you a small performance cases if you have to output a lot of strings:

std::ios::sync_with_stdio(false);

Remove the unnecessary function

You are not using myFunction anywhere in your program, so you can safely remove it.

You don't need return 0;

You don't need to return 0; from main, as the compiler will implicitly do it for you. Although sometimes, it doesn't harm specifying it, when for example you are returning with an error in main, and want to show that the function was a success.

You can use emplace_back

Instead of creating a voter, filling it with data, and the copying it to the vector, you can directly construct it in the vector using emplace_back:

voters.emplace_back(name, 1);

Note that this will require you to write a constructor for voter.

Source Link
Rakete1111
Rakete1111
  • 2.6k
  • 1
  • 15
  • 23