2

I was trying to get the maximum length of a string from an array of strings.

I wrote this code:

char a[100][100] = {"solol","a","1234567","123","1234"};
int max = -1;
for(int i=0;i<5;i++)
    if(max<strlen(a[i]))
    max=strlen(a[i]);
cout<<max;

The output it gives is -1. But when I initialize the value of max by 0 instead of 1, the code works fine. Why is it so?

Aconcagua
  • 24,880
  • 4
  • 34
  • 59
Muktadir Khan
  • 148
  • 3
  • 17
  • 5
    Why `char[][]` and not `std::vector>`? The latter would be super easy to just use `std::max_element` and is more C++ style. – Cory Kramer Mar 26 '18 at 14:52
  • 1
    The data type of `max` should be `size_t`. – H.S. Mar 26 '18 at 14:53
  • @CoryKramer Just answer my question instead of suggesting me to use vectors. I know I can use vectors but why does this doesn't work? – Muktadir Khan Mar 26 '18 at 14:54
  • Removed the c-Tag as title clearly states "C++"... – Aconcagua Mar 26 '18 at 14:54
  • 1
    Are you getting a warning about comparing signed and unsigned types? Because -1 might be much larger than 500. – Kenny Ostrom Mar 26 '18 at 14:55
  • 1
    [Signed/unsigned comparisons](//stackoverflow.com/q/5416414) – 001 Mar 26 '18 at 14:55
  • @H.S. is right, it is likely due to `max – vasilyrud Mar 26 '18 at 14:55
  • Well, you managed to fool me with the horrible indention. Maybe adopt a sane and safe coding style, by always using `{ }` and indention. – Lundin Mar 26 '18 at 14:58
  • Possible duplicate of [Implicit type promotion rules](https://stackoverflow.com/questions/46073295/implicit-type-promotion-rules). – Lundin Mar 26 '18 at 14:58
  • @Lundin I purposely skip { } when I know beforehand that there will be a single statement in the block. – Muktadir Khan Mar 26 '18 at 15:03
  • `std::max_element` can still be used, as demonstrated [here](http://coliru.stacked-crooked.com/a/9f93a9d2ef6b3c27) – PaulMcKenzie Mar 26 '18 at 15:46
  • @MuktadirKhan Leaving the braces is not compatible with *almost* any coding conventions, and not with *really* any authoritative ones (such as MISRA). Long experience of thousands of coders, so many bugs when adding another instruction and forgetting the braces... – Aconcagua Mar 26 '18 at 17:21
  • @MuktadirKhan Yeah, don't do that, it is bad practice. – Lundin Mar 27 '18 at 06:53

5 Answers5

6

The function strlen returns an unsigned integral (i.e. size_t) so when you compare it to max which is signed, it gets promoted to unsigned and that -1 turns to something like 0xffffffff (the actual value depends on your architecture/compiler) which is larger than any of those strings.

Change max type to be size_t and never compare unsigned with signed integrals.

imreal
  • 10,178
  • 2
  • 32
  • 48
2

size_t is the unsigned integer type.

From C++ Standard#4.6:

A prvalue of an integer type other than bool, char16_t, char32_t, or wchar_t whose integer conversion rank (4.15) is less than the rank of int can be converted to a prvalue of type int if int can represent all the values of the source type; otherwise, the source prvalue can be converted to a prvalue of type unsigned int.

The value of max is -1 which represent the maximum value that an unsigned integer type can hold. In the expression max<strlen(a[i]), the value returned by strlen will be promoted to unsigned int and will be compared with the maximum value of the unsigned integer. Hence the condition max<strlen(a[i]) will never true for the given input and the value of max will not change.

The longest string in the array of strings a is of length 7. When you initialize the max with 0 or 1, the expression max<strlen(a[i]) evaluate to true for any string whose length is greater than the value of max. Hence you will get the expected output.

H.S.
  • 11,654
  • 2
  • 15
  • 32
1

When evaluating expressions, the compiler breaks each expression down into individual subexpressions. The arithmetic operators require their operands to be of the same type. To ensure this, the compiler uses the following rules:

If an operand is an integer that is narrower than an int, it undergoes integral promotion (as described above) to int or unsigned int. If the operands still do not match, then the compiler finds the highest priority operand and implicitly converts the other operand to match. The priority of operands is as follows:

long double (highest) 
double
float
unsigned long long
long long
unsigned long
long
unsigned int
int (lowest)

In your case on of operands has type int and another one type size_t, so max was promoted to type size_t, and bit representation of -1 is the biggest possible size_t.

You might also want to look in the following parts of the Standard [conv.prom], [conv.integral], [conv.rank].

Yola
  • 18,496
  • 11
  • 65
  • 106
  • @Lundin thanks, for your comment. i would need to cite quite a lot from the Standard in this case. And Standard is not so easy to read. – Yola Mar 26 '18 at 15:14
1

strlen returns an unsigned int and when doing the comparison -1 gets promoted to a maximum value an unsigned can hold.

To fix the code, change the max definition to unsigned int max = 0

nVxx
  • 661
  • 7
  • 20
0

I think this might work for you.

#include <algorithm>

  const size_t ARRAYSIZE = 100;
char stringArray[ARRAYSIZE][100] = {"solol","a","1234567","123","1234"};

auto longerString = std::max_element(stringArray, stringArray + ARRAYSIZE - 1, [](const auto& s1, const auto& s2){
      return strlen(s1) < strlen(s2);
    });

size_t maxSize = strlen(longerString[0]);

This works for me, returns the longest char array, just do a strlen over the "first element" and youre done. For your example, it returns 7.

Hope it helps.

Jonathan Michel
  • 19
  • 5