1

This is part of a lab assignment

I have to implement the following function...

void replaceChar(char s[], char c,int len)

Description: Replace every character of s with c. len indicates the length of s.

I submit this to the autograder that my class uses and it tells me that "the length of the replaced string has different length." I have tested this extensively and do not see any issues. Here is my complete function:

void replaceChar(char s[], char c, int len) {
    printf("\n");
    for (int i = 0; i < len; i++) {
        s[i] = c;
        printf("%c",s[i]);
    }
}

I appreciate any help you can give me!

Here are a few of my test cases:

char s1[5] = {'h','e','l','l','o'};

char s3[10] = {'h','e','l','l','o',' ','h','i','i','i'};
char rep1 = 'x';
replaceChar(s1,rep1,5);
replaceChar(s3,rep1,10);
slugster
  • 49,403
  • 14
  • 95
  • 145
Matt Altepeter
  • 317
  • 2
  • 5
  • 17
  • What are you passing for `len`? – obataku Sep 09 '12 at 19:53
  • Maybe talk to the teacher or a TA? – Brendan Long Sep 09 '12 at 19:53
  • 3
    I suspect you are misinterpreting the len parameter – Paul R Sep 09 '12 at 19:53
  • You need to consider exactly how strings are encoded in C. Is this a cstring or a bstring? – Alex Brown Sep 09 '12 at 19:54
  • is the last element of that array "null" ? – huseyin tugrul buyukisik Sep 09 '12 at 19:54
  • Q: "Here is my complete function". A: Your *complete* code would include the part that *calls* "replaceChar()". The problem is quite possibly with your "len" parameter. Please add 1) your calling code, 2) where "len" is assigned, and 3) where "s[]" is declared. – paulsm4 Sep 09 '12 at 19:55
  • Found the lab assignment [here](http://www.cs.purdue.edu/homes/cs240/labs/lab2.pdf). "*Description:* Replace every character of `s` with `c`. `len` indicates the length of `s`." – obataku Sep 09 '12 at 19:55
  • @paulsm4 he's not passing anything... that all is done by whatever he submits his code to. – obataku Sep 09 '12 at 19:55
  • 2
    @MattAltepeter have you tried explicitly marking the NUL terminator? `s[len] = '\0';` – obataku Sep 09 '12 at 19:59
  • Maybe the `printf`s are messing up the autograder ? – Paul R Sep 09 '12 at 20:00
  • Well I do have a tester.c program to test all of my utility functions. @pb2q I know the autograder does have problems with that, so i tried removing it but it didnt help at all – Matt Altepeter Sep 09 '12 at 20:01
  • @MattAltepeter it's probably better to use `char s1[] = "hello";` rather than what you have now. Same for `s3`. This way they're guaranteed to be NUL-terminated. – obataku Sep 09 '12 at 20:02
  • 1
    Thank you @oldrinb ! I added that after my loop and resubmitted it and it removed the error! Thanks a bunch! – Matt Altepeter Sep 09 '12 at 20:03
  • Ironically, storing that NUL introduces a bug that *should* produce the given error message in a properly coded autograder instead of eliminating the error message. – Jim Balter Sep 09 '12 at 20:16
  • @JimBalter why would it *produce* the bug? If the autograder were correct it would merely set what is already the terminator to NUL. – obataku Sep 09 '12 at 20:22
  • @oldrinb See my comment to your answer ... instead of "hexxo" it would produce "hexx", changing the length of the string. I would write an autograder (or unit test) with such a test to make sure that replaceChar is only doing what it's supposed to do and not storing outside of s .. s+len-1 – Jim Balter Sep 09 '12 at 20:29

1 Answers1

2

See my comment:

have you tried explicitly marking the NUL terminator? s[len] = '\0';

Though this corrects the issue, I'd still get in contact with your professors and ask them why this was necessary in the first place. The lab does not specify that the string s needs to be NUL-terminated.

As an aside (mentioned originally in another of my comments), you should take advantage of using string literals when initializing s1 and s3, i.e.

char s1[] = "hello";
char s3[] = "hello hiii";

This not only looks much cleaner (and avoids the need to explicitly mention the array length) but also guarantees the strings are NUL-terminated to begin with, too.

Community
  • 1
  • 1
obataku
  • 29,212
  • 3
  • 44
  • 57
  • `s[len] = '\0'` goes beyond the specification. It's quite possible to pass to replaceChar a pointer internal to a string, or a buffer that isn't NUL-terminated. The bug here is actually with the test framework, not the OP's implementation of replaceChar. – Jim Balter Sep 09 '12 at 20:14
  • @JimBalter I stated that the specification didn't necessitate it, and that he should ask his professor (see ¶2). PS what do you mean by `a pointer internal to a string`? – obataku Sep 09 '12 at 20:24
  • e.g,., `replaceChar(s1+2, 'x', 2)` should change "hello" to "hexxo". – Jim Balter Sep 09 '12 at 20:27
  • @JimBalter that's misleading; the specification states `len` is the *length of `s`*, not just the amount of characters to replace. In that case it is the caller's fault for lying about the length of the string. – obataku Sep 09 '12 at 20:29
  • s is defined as char[] ... length is the length of s. In C (and in utility routines like this), it is common for s to be a substring of a larger string ... that's the whole point of passing a length rather than using the NUL terminator. Note that both strlen and strNCmp specify that the arguments are strings, whereas replaceChar does not. Anyway, the instructor gets poor marks for imprecise specification. – Jim Balter Sep 09 '12 at 20:33
  • @JimBalter you're making presumptions that are not stated in the specification. `s1 + 2` is a pointer to the interior of a string, rather than a string itself. I understand the point that your intuition is valid *common sense*, but it's not necessarily valid -- especially in the land of bugs :-) – obataku Sep 09 '12 at 20:37
  • Um, pot/kettle/black. You seem not to understand what a char[] is in C; it is not necessarily a NUL-terminated string -- you assumed it, I did not. (BTW, my answer suffers the same problem, but I hadn't seen the lab specification at the time.) – Jim Balter Sep 09 '12 at 20:40
  • I didn't assume that `char[]` is a string. The function is part of his larger "String Utils" library, part of the lab assignment I had found. `replaceChar` specifies it is expected to work given `len` as the *length* of a string; as specified for `strLen`, "*A null terminated string is an array of characters with a `\0` character at the end. The `\0` character is not counted, so the length of `"abc"` is `3`.*" This is the definition of *length* I used to interpret the `len` parameter, and it makes perfectly valid sense. – obataku Sep 09 '12 at 20:44
  • 'I didn't assume that char[] is a string' -- untrue. Goodbye. – Jim Balter Sep 09 '12 at 20:45
  • 1
    Jim Balter is right in that _s_ could point inside a larger char[]. NULL terminating the string in the function should never be done. It makes the whole function useless. Whether or not the char[] is NULL terminated is determined by whoever sends you a pointer to it (offset or not), and the function shouldn't change that. If it wasn't NULL-terminated (ex. a substring was essentially passed), it should leave not NULL-terminated, not **** up the string by effectively truncating it to s+len. The test framework at use here is appalling, and the question, unclear. – Metabble Sep 09 '12 at 21:23
  • @Metabble nice, but I disagree. The lab specification is for strings, where `len` is the *length* of s -- defined as the number of characters before the NUL-terminator. Nowhere does the function have to work for `s` such that `len` is actually not the length of the string. I agree that this seems a bit stupid on their part, but it's within the lab specification to do what is done here. Again, I don't necessarily advocate this (hence why I advised him to speak to his professor), but it fits the specification. – obataku Sep 09 '12 at 21:29
  • 1
    @oldrinb Where is len defined as the length before the NULL character? You can make a leap by connecting unrelated functions, but the literal meaning doesn't specify. Looking at the grading criteria it says: _replaceChar() - replaces every character of string s, up to length n, with a provided character._ Going by that technical definition your solution oversteps its bounds. Yes, s is a string. Do we know where n/len ends? How do we know it leads us to the end, and not the middle? Your solution _technically_ replaces every character of string s up to n with c... then replaces s[len] with NULL. – Metabble Sep 09 '12 at 21:37
  • 1
    Also, just to clarify, I think that your solution is what they were looking for (stupidly enough), I just think the spec could've been a lot clearer, since normally you wouldn't assume you should just add a NULL. It's natural to think you don't need it; in real life, when would you add it? It lowers the usefulness of the function. EDIT: Also, why did his version fail? If the test framework passes a string in as it says it does, not sticking a NULL in there won't change the length; s should already contain one at the end. Confusing. – Metabble Sep 09 '12 at 21:39
  • @Metabble I agree with you that it should've been far more clear, and is a rather poor design choice. I don't, however, agree that I'm connecting unrelated functions... `strLen` defines what they mean by *length* of a string. I merely applied that definition to the interpret the following usages :-) Anyways, I'm just glad the OP's work was accepted. I believe the reason his failed is because the testing framework passed in a string lacking the terminator and used a hard-coded length. I __really__ think he should bring this up to his professor, even if just to ensure others are graded fairly. – obataku Sep 09 '12 at 21:43
  • it's not so far-fetched to do that, you are correct, but it's still unclear. What really confuses me is... what could they pass to test this that it would fail and change the length when he didn't add a NULL terminator? Not a string like they say, as that, going by your definition of length as strLen, would make them the same length. O_e EDIT: Ah. Yeah, maybe. Wait. So they were stupid enough to pass in a char[] without a NULL after saying it was a string, hard code the length, then try to use something like strLen to get the length, which they can't since they forgot the NULL? lol. – Metabble Sep 09 '12 at 21:46
  • @Metabble yeah, it's quite strange... I'm just trying to rationalize why it was designed like it was, taking into consideration the observed behavior. Honestly, your guess as well as Jim's are rightfully as good as mine -- in fact, maybe even better! – obataku Sep 09 '12 at 21:50
  • @oldrinb What scares me is that you'd expect the people teaching others to be pretty competent, but that's often not case. Me, I would've tested it myself (since it's such a simple thing) with a few answers I would consider valid. The person who wrote the test framework probably didn't bother to use string literals himself, lol. – Metabble Sep 09 '12 at 21:56
  • @Metabble more than likely not... well, you can't safely modify the contents of string literals, can you? – obataku Sep 09 '12 at 22:05
  • @oldrinb True. I hadn't thought of that (I often forget). This is why I'm not a teacher! xD That and that I only spent a little over a month learning C before moving onto Objective-C. >_> Just enough to write a small RPG battle engine whose source would probably make any experience C programmer cringe. :p – Metabble Sep 09 '12 at 22:17
  • Given the description quoted by @Metabble, I'd have expected the error was that they had a testcase along the lines of `char str[20] = "hello"; replaceChar(str, 'x', 17);` where the OP's function overwrites the 0-terminator and thus changes the length. – Daniel Fischer Sep 10 '12 at 14:04