WString.h: Add const qualifier to begin and end functions

[Ivan-Perez]

No description provided.

[matthijskooijman]

Looks good to me!

[Chris--A]

@matthijskooijman

Actually, after the PR where I implemented this was added, I realized they should not be const at all!.

The functions really need to be:

char* begin() { return buffer; }
char* end() { return buffer + length(); }

The ranged loop shouldn't be read-only. It doesn't make sense to force users to use a standard for loop and subscript operator to be able to write elements (str[0] = 'a';).

Currently the code below fails, and should be working:

  String str = "test";

  for( char &c : str ){
    c = 'a';
  }

[Ivan-Perez]

Without const, this simple sketch fails to compile:

void test(const String &t)
{
  for (char c : t) {// passing 'const String' as 'this' argument of 'const char* String::begin()' discards qualifiers
    Serial.print(c);
  }
}

void setup() {}
void loop() {
  test(F("sdfsdf"));
}

Do you know if it is possible to add both functions, and let the compiler select which one to use?

[Ivan-Perez]

Anyway, the signature of the begin function now is const char* begin(). Note that a const is returned, so is it correct to modify the chars in the iterator? What would happen if you assign '\0' to one of those chars? Will the string length be updated accordingly?

[Chris--A]

@Ivan-Perez

Yes, with both versions of the functions, both examples compile fine. Looks like a good compromise.

char* begin() { return buffer; }
char* end() { return buffer + length(); }
const char* begin() const { return buffer; }
const char* end() const { return buffer + length(); }

What would happend if you assign '\0' to one of those chars? Will be the string length updated accordingly?

The operator[] includes both methods (read/write), so this should follow suit. The problem you have mentioned is why I have this PR: #3096

Also, @matthijskooijman has mentioned in there the expectation to be able to store a null mid string.

[Ivan-Perez]

@Chris--A OK, tried it and it works perfectly. May I update the pull request to add the non-const functions?

[Chris--A]

Yeah, just in-case, the non const versions cannot use c_str() due to it being const, they should use buffer instead.

[Chris--A]

@Ivan-Perez

I have added a PR to your branch patch-1 which adds these changes to the SAM core as well.
Ivan-Perez#1

[Ivan-Perez]

@Chris--A Merged!

[Chris--A]

This is looking good!

Just needs accepting by the big wigs now.

[cmaglie]

Rebased and merged. Thanks you!