Skip to main content
Spelling fix
Source Link
Toby Speight
  • 81.7k
  • 14
  • 101
  • 308

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabitalphabetical content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?

These are good questions to ask your customer! :-)

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabit content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?

These are good questions to ask your customer! :-)

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabetical content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?

These are good questions to ask your customer! :-)

added 71 characters in body
Source Link
Loki Astari
  • 96.6k
  • 5
  • 125
  • 338

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabit content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?

These are good questions to ask your customer! :-)

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabit content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?

These are good questions to ask your customer! :-)

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabit content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?
What happens if there are ties in the number of duplicate counts?

These are good questions to ask your customer! :-)

Source Link
Loki Astari
  • 96.6k
  • 5
  • 125
  • 338

Nothing awful here.

If this was production code then putting things in named functions would be nice to help with self documenting code.


Don't particularly like this.

    using std::cin; using std::cout; using std::endl;
    using std::string;

But I am not going to complain about it very much.


Using snake case is unusual but not unheard of in C++.

    string most_duplicated_str;
    int most_duplicated_count = 0;

Like the long descriptive names and the grouping of you variables into logical combinations.


I would note that:

      cin >> new_str

Reads a white space separated word. For college and learning purposes this will be fine. But in real life you would have to consider punctuation and other non alphabit content and what to do with it.


Not a very useful comment.
I would either make it more descriptive (more words) or remove it.

    // loop variables

and

            // update prev_str

These three lines can be simplified:

            new_duplicate_count = 1;
            prev_str = new_str;
            continue;

to:

            new_duplicate_count = 0;

It will have the same effect and the loop will feel more logical as the same action happens every loop. But that's just my opinion on readability. If you did not change it I would not oppose it being merged into mainline.


If any words are read: Can the value ever be zero?

    // print result
    if(most_duplicated_count == 0)
        cout << "# No Duplicated Words #\n";
    else

If you only have a duplicate count of 1 were there any duplicates?

These are good questions to ask your customer! :-)