JohnIsStupid
JohnIsStupid

Reputation: 21

Why is it that my code is only showing the last element in the array even though It should be showing the element with the most amount of characters

#include <iostream>
#include <vector>
#include <ctime>
using namespace std;

vector<string> createvector() {
    vector<string> words;
    string names;
    cout << "Please enter 5 different words: " << endl;
    for (int i = 0; i < 5; i++) {
        cin >> names;
        words.push_back(names);
    }
    return (words);
}
void mostchar(vector<string> words) {
    string w1 = words[0];
    string largestword;
    for (int i = 1; i < 5; i++) {
        if (words[i] > w1) {
            largestword = words[i];
        }
    }
    cout << "The largest word is: " << largestword;
}

int main()
{
    vector<string> words;
    string names;
    words = createvector();
    mostchar(words);
}

I do not understand why it's picking the last element or the second to last element every time. Right I've tried to change for(int i = 1; i < 5; i++) but it makes no difference to what I do.

Upvotes: 1

Views: 213

Answers (2)

Austin
Austin

Reputation: 2265

There are a couple issues here:

1: You should use something like .length() to compare "length"

2: You are comparing the next word in the array to words[0] every time.

EDIT: To further explain this, there is an assignment of string w1 = words[0];. w1 is then used in the if in the for loop here:

string w1 = words[0];
string largestword;
for (int i = 1; i < 5; i++) {
    if (words[i] > w1) {
        largestword = words[i];
    }
}

resulting in the value of words[0] being the value repeatedly compared in the loop.

Adjust the comparison line to if (words[i].length() > largestword.length()) and that solves both problems. You can elminate w1 entirely this way as well.

#include <iostream>
#include <vector>
#include <ctime>
using namespace std;

vector<string> createvector() {
    vector<string> words;
    string names;
    cout << "Please enter 5 different words: " << endl;
    for (int i = 0; i < 5; i++) {
        cin >> names;
        words.push_back(names);
    }
    return (words);
}
void mostchar(vector<string> words) {
    string largestword;
    for (int i = 0; i < 5; i++) {
        if (words[i].length() > largestword.length()) {
            largestword = words[i];
        }
    }
    cout << "The largest word is: " << largestword;
}

int main()
{
    vector<string> words;
    string names;
    words = createvector();
    mostchar(words);
}

Upvotes: 1

Vlad from Moscow
Vlad from Moscow

Reputation: 310970

For starters you are comparing strings in the lexicographical order.

if (words[i] > w1) {

Secondly you always comparing with the word in the first element of the array

if (words[i] > w1) {

and the variable w1 is not being changed within the loop. So any last element in the vector that is greater than w1 will be assigned to the variable largestword.

Using the for loop the function can look the following way

void mostchar( const std::vector<std::string> &words ) 
{
    size_t largestword = 0;

    for ( size_t i = 1; i < words.size(); i++ ) 
    {
        if ( words[largestword].size() < words[i].size() ) 
        {
            largestword = i;
        }
    }

    if ( largestword != words.size() )
    {
        std::cout << "The largest word is: " << words[largestword] << '\n';
    }
}

Pay attention to that in general case the user can pass to the function an empty vector. You must check such a possibility within the function.

Bear in mind that there is standard algorithm std::max_element that can be used instead of manually written for loop.

For example

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

void mostchar( const std::vector<std::string> &words ) 
{
    auto largestword = std::max_element( std::begin( words ), std::end( words ),
                                         []( const auto &a, const auto &b )
                                         {
                                             return a.size() < b.size();
                                         } );

    if ( largestword != std::end( words ) )
    {
        std::cout << "The largest word is: " << *largestword << '\n';
    }
}

Upvotes: 2

Related Questions