0

User should enter a few strings and input space as string when he is done. Code should return longest and shortest word entered.

strcmp always returns -1... what am i doing wrong?

#include <iostream>
#include <cstring>
using namespace std;
int main() {
    char S[100][20];
    int I = 0;
    do {
            cout << "Enter text:" << endl;
            cin.getline(S[I],100);
    } while (I < 19 && strcmp(S[I++],""));
    char Max[100], Min[100];
    strcpy(Max, S[0]);
    strcpy(Min, S[0]);
    for (int J = 1; J < I; J++) {
        if (strcmp(S[J], Max) == 1)
            strcpy(Max, S[J]);
        if (strcmp(S[J], Min) == -1)
            strcpy(Min, S[J]);
    }
    cout << "Max = " << Max << endl;
    cout << "Min = " << Min << endl;
    system("pause");
    return 0;
}
Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
eqagunn
  • 221
  • 1
  • 4
  • 11

5 Answers5

2

Try use char S[20][100]; instead of char S[100][20];

kochobay
  • 392
  • 1
  • 7
2

So, a couple of things:

  • variables should be lowercase;
  • you are defining your array of string with wrong length (should be s[20][100]);
  • in your while cycle, you should go 'till i < 20;
  • the last string in your array will always be the empty string (hence: s_min will be always empty);
  • strcmp compares strings, it doesn't tell you which one is the longest. You should use strlen for that...

Here the working code:

#include <iostream>
#include <cstring>
using namespace std;

int main() {
  char s[20][100];
  int i = 0;
  do {
    cout << "Enter text:" << endl;
    cin.getline(s[i], 100);
  } while (i < 20 && strcmp(s[i++],""));

  char s_max[100], s_min[100];
  strcpy(s_max, s[0]);
  strcpy(s_min, s[0]);
  for (int j = 1; j < i-1; j++) {
    if (strlen(s[j]) > strlen(s_max))
      strcpy(s_max, s[j]);
    if (strlen(s[j]) < strlen(s_min))
      strcpy(s_min, s[j]);
  }

  cout << "Max = " << s_max << endl;
  cout << "Min = " << s_min << endl;
  return 0;
}
StefanoP
  • 3,798
  • 2
  • 19
  • 26
  • 1
    Variables should be lowercase? That is entirely irrelevant, at best, and at odds with typical practice for C++, at worst. – Sion Sheevok Jan 23 '12 at 19:59
  • it is irrelevant for the compiler, it is important for code readability. I sincerely get an headache when reading sources that don't follow standards. just my opinion, though.. – StefanoP Jan 23 '12 at 20:03
  • mm, so if we talk about code redability, we should also consider that variable should have significant name, so `char strings[20][100]` instead of `char s[20][100]`. Anyway, in my opinion it doesn't have such relevance in this context, when developing a large scale application surely, but this is just an exercise. – Dan Tumaykin Jan 23 '12 at 20:11
  • Thank you, although I already had it working with strlen. The assignment said to use strcmp, so that why I used it instead of strlen, but couldn't make it work. – eqagunn Jan 23 '12 at 20:17
  • okay, that makes sense... I completely agree with @dtumaykin (and there are so many other things to say about code clarity, using constants or `#define` for example), but uppercase variables are just something I can't stand... – StefanoP Jan 23 '12 at 21:12
1

From cplusplus.com:

Returns an integral value indicating the relationship between the strings: A zero value indicates that both strings are equal. A value greater than zero indicates that the first character that does not match has a greater value in str1 than in str2; And a value less than zero indicates the opposite.

It shouldn't be 1 or -1, try testing with >/< 0. Also, test if the strings read from stdio ends up in '\0' and eventually add it(getline should), because strcmp makes use of it.

Also, array size if wrong. Should be char S[20][100].

Dan Tumaykin
  • 1,213
  • 2
  • 17
  • 37
  • Ok, I changed it to >/< 0, and fixed the array size. But if I enter John, David and Leo for strings, first strcmp will still return -1, although it should obviously return positive value. And I don't know how to check for /0. – eqagunn Jan 23 '12 at 19:38
  • getline() adds null char to the end -- looking up for the rest now – Dan Tumaykin Jan 23 '12 at 19:44
  • works for me on VS2010, which compiler are you using? also, mind decreasing I, when you're done with input - the last string shouldn't be considered – Dan Tumaykin Jan 23 '12 at 19:50
  • I'm using Bloodshed Dev-C++ (this is MinGW/GCC i think). Yeah, "I" should be decreased by one. I made it work using strlen, but using strcmp still doesn't work for me. – eqagunn Jan 23 '12 at 19:55
  • Try to use the code @StefanoP proposes here. If neither it works well, try to update or change IDE. Unfortunately I'm familar with Dev-C++(used it at school) and I can say it isn't *khm* the best choice. Try VS2010 Express C++, it's free and way more decent then Dev-C++. – Dan Tumaykin Jan 23 '12 at 20:01
0
  1. You're declaring your array backwards; it looks like you really want char S[20][100].

  2. You need to compare using < 0 and > 0, since strcmp() doesn't guarantee that it will return you a 1 or a -1. From the man page:

    The strcmp() and strncmp() functions return an integer less than, equal to, or greater than zero if s1 (or the first n bytes thereof) is found, respectively, to be less than, to match, or be greater than s2.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469
0

You're trying to compare the strings in your code but your question says you want to compare length. Use strlen on a null-terminated strings to compare lengths. Here is the reference. Better yet, if what you're writing is C++ code (as it is tagged), you're already using the standard library, so go ahead and use std::string. This sounds like it should be tagged homework, however, in which case I assume you can't use the std::string class.

Sion Sheevok
  • 4,057
  • 2
  • 21
  • 37