Skip to main content
If people want to know what you edit, they would look at the revision history. Writing it in the post is very unnecessary and doesn't look good.
Source Link

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }

Keep in mind, however, that this constructor allows implicit conversions from integer types, which you typically want to avoid. This is easily prevented by making the constructor explicit.

  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.

Edit: The link to the style guide now points to the C++ core guidelines. Also, I added a paragraph about explicit to point 3.

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }

Keep in mind, however, that this constructor allows implicit conversions from integer types, which you typically want to avoid. This is easily prevented by making the constructor explicit.

  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.

Edit: The link to the style guide now points to the C++ core guidelines. Also, I added a paragraph about explicit to point 3.

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }

Keep in mind, however, that this constructor allows implicit conversions from integer types, which you typically want to avoid. This is easily prevented by making the constructor explicit.

  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.
Changed link and added information in compliance with @Deduplicator's comment
Source Link
Ben Steffan
  • 5.3k
  • 1
  • 18
  • 35

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guideC++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }

Keep in mind, however, that this constructor allows implicit conversions from integer types, which you typically want to avoid. This is easily prevented by making the constructor explicit.

  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.

Edit: The link to the style guide now points to the C++ core guidelines. Also, I added a paragraph about explicit to point 3.

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }
  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }

Keep in mind, however, that this constructor allows implicit conversions from integer types, which you typically want to avoid. This is easily prevented by making the constructor explicit.

  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.

Edit: The link to the style guide now points to the C++ core guidelines. Also, I added a paragraph about explicit to point 3.

Source Link
Ben Steffan
  • 5.3k
  • 1
  • 18
  • 35

Just the general C++ coding style treatment.

  1. Do not use using namespace std;. It can introduce subtle bugs, makes the code harder to read and writing out the std:: prefix generally is not going to be to time intensive.

  2. Keep your spacing consistent. Compare the following two lines

    bool operator< (const Node& other)
    vector<int> topKFrequent(vector<int>& nums, int k)
    

Why is one method name followed by a space while the other is not? Consistency is important! If you are not sure which spacing variant to choose, you should read a C++ style guide. The same applies to control structures, such as

    for (auto n : nums )
    if ( frequency.find(n) != frequency.end())
    for ( auto f : frequency )
  1. Use a member initializer list in your constructor. Although it does not matter in a lot of cases, it is a more concise way of expressing member initialization and leaves the constructor's body open for more complex setup tasks. The Node constructor would then look something like this:

    Node(int value, unsigned frequency) : value(value), frequency(frequency) { }
    

Also, since you default constructor does not actually do anything, you should remove it and the initializers for value and frequency and add default values to the constructor parameters instead, so that Node's constructor becomes something like

    Node(int value = 0, unsigned frequency = 0) : value(value), frequency(frequency) { }
  1. Most people prefer to not write this-> explicitly when not needed.
  2. const-correctness is important. You should take the parameter nums in topKFrequent by const-reference since you do not actually modify it.
  3. Always use curly brackets with if,else,for,while etc. This prevents a lot of errors from statements not being included in conditionally dependent blocks when they should be. In fact, this kind of error is so common that gcc even introduced a diagnostic for misleading indentation in version 6.