5
\$\begingroup\$

I got a word count assignment and the requirements are shown below:

INPUT

The input contains words in several lines and in each line there could be more than one word. The input only contains English words, separated by space or line breaks. Words are case-sensitive.

OUTPUT

Print the words in alphabetic order with the corresponding appearance count in the text. For each word, please print the result in a single line (separate the word and the number with space).

Sample Input:

Computer system
computer design
algorithm design and analysis
quantum computer
computer science Department

Sample Output:

Computer 1
algorithm 1
analysis 1
and 1
computer 3
department 1
design 2
quantum 1
science 1
system 1

I tried something shown below and actually got the same output as the sample but still got marked as wrong answer by the assignment judge system.
Is there anything I did wrong?

#include <iostream>
#include <algorithm>
#include <string>
#include <sstream>
#include <iterator>
#include <vector>
using namespace std;

int main() {

    string whole;
    string line;
    string word;

    do {
        getline(cin, line);
        whole += line + ' ';
    } while (!line.empty());

    if (!whole.empty()) {
        istringstream iss(whole);
        vector<string> results((istream_iterator<string>(iss)), istream_iterator<string>());

        int count = 1;
        int size = results.size();
        if (size == 1) cout << results[0] << ' ' << count;
        else {
            sort(results.begin(), results.end());
            for (int j = 0; j < size; j++) {
                if (j != size - 1) {
                    if (results[j] != results[j + 1]) {
                        string word = results[j];
                        cout << word << ' ' << count << endl;
                        count = 1;
                    }
                    else if (results[j] == results[j + 1]) count++;
                }
                else if (j == size - 1) {
                    if (results[j] != results[j - 1]) {
                        string word = results[j];
                        cout << word << ' ' << count << endl;
                    }
                    else if (results[j] == results[j - 1]) cout << results[j] << ' ' << count << endl;
                }
            }
        }
    }
    return 0;
}
\$\endgroup\$
3
\$\begingroup\$
  1. Kudos for proper formatting. And for including the headers you use, but no more.

  2. Don't use using namespace std;. That namespace is not designed to be imported wholesale, thus doing so can have surprising and unpredictable results, which aren't even stable for minor changes in the implementation. Obvious conflicts, and silent changes are possible.

  3. You are reading lines from the stream, just to turn around and put them into a string-stream from which you then read words?
    Doing such contortions is sub-optimal.

  4. Using uniform-initialization allows you not to repeat the iterator-class-name for the default-constructed second argument.

  5. There is no reason to special-case the input consisting of just one word.

  6. Use a for-range-loop instead of a classic for-loop. Fumbling around with indices is error-prone.

  7. If you need to do the same thing twice, consider encapsulating it in a function, or at least a lambda. Repetition is also error-prone.

  8. return 0; is implicit in main().

  9. Finally, if the same words are used very often, it might be a good idea to directly compute frequency in a std::map or std::unordered_map instead of reading all words, sorting, and then collating the input.
    I will not do so here.

Doing all that, and more cleanup:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>

int main() {
    std::vector<std::string> words(std::istream_iterator<std::string>(std::cin), {});
    std::sort(begin(words), end(words));

    auto p = words.data();
    auto n = 0ULL;
    auto output = [&]{ std::cout << *p << ' ' << n << '\n'; };

    for (auto&& x : words) {
        if (x == *p) {
            ++n;
        } else {
            output();
            n = 1;
            p = &x;
        }
    }
    if (n)
        output();
}

Considering it was an assignment for a computing-course, not demonstrating your understanding whatever lessons you had to date might have been a reason for marking you down.

\$\endgroup\$
4
\$\begingroup\$

@Deduplicator said almost everything, I will only add that it's really important that you know algorithms and data structures from the STL and using the proper in each case. Here, you do so much work for nothing:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <string>
#include <map>

int main() {
    auto in = std::istream_iterator<std::string>{std::cin};
    auto out = std::map<std::string, int>{};

    std::for_each(in, {}, [&out](const std::string& w) { out[w]++; });
    for (auto& w : out) { std::cout << w.first << ' ' << w.second << '\n'; }
}
\$\endgroup\$
  • \$\begingroup\$ Good answer. I'd suggest that a std::unordered_map might confer a performance advantage over a std::map in this application. \$\endgroup\$ – Edward Oct 24 '18 at 2:00
  • 1
    \$\begingroup\$ since he asked for an alphabetically ordered list of words as output, using a normal map is more appropriate. \$\endgroup\$ – Calak Oct 24 '18 at 9:17
  • \$\begingroup\$ You’re right. I had overlooked the sorting requirement. \$\endgroup\$ – Edward Oct 24 '18 at 10:20
  • \$\begingroup\$ There might be a significant performance-boost to using the unordered_map for data-entry, then getting a vector and sorting. But yes, it wouldn't be quite so nice and short. \$\endgroup\$ – Deduplicator Oct 24 '18 at 10:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.