comment on my solution to exercise 3.3 in K&R's The C programming language

[Jason]

hello everyone! recently i've been working through K&R's The C
programming language, and i just finished exercise 3.3, since my
solution is shorter than the "suggested solution" online, i want to
see if mine is correct to spec.

The question is as follows: Exercise 3-3. Write a function
expand(s1,s2) that expands shorthand notations like a-z in the string
s1 into the equivalent complete list abc...xyz in s2. Allow for
letters of either case and digits, and be prepared to handle cases
like a-b-c and a-z0-9 and -a-z. Arrange that a leading or trailing -
is taken literally.

One question before going on to the code, i interpret their "leading
or trailing - is taken literally" by that for example, if i input -a,
the program would expand from '-' all the way to a based on ascii
character set, and similar thing would happen for trailing -. However,
there are certain ambiguity with the following sample input: "-a-z",
my program would interpret that as from '-' to 'a' then from '-' to
'z', doesn't seem right, but that's the simplist way of doing it.

Anyways, here is my code, blast away!

#include <stdio.h>

/*expands shorthand notations like a-z in the string s1 into the
equivalent
complete list abc...xyz in s2. Allow for letters of either case and
digits,
and be prepared to handle cases like a-b-c and a-z0-9 and -a-z.
Arrange that
a leading or trailing - is taken literally.*/
void expand(char s1[], char s2[])
{
int i, j, queue, start, end;
j = queue = start = end = 0;
for(i = 0; s1[i] != '\0'; i++)
{
if(s1[i] == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1[i]; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1[i]; /* handles trailing '-' */
}else{
end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1[i];
}
}
}

main()
{
char str1[] = "a-zA-Z";
char str3[] = "-a-zA-Z";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand(str3, str2);
printf("Input: %s\n", str1);
printf("Output: %s\n", str2);
system("pause") ;
}
Any comments would be much appreciated!

[Jason]

Slightly modified to handle the case of a-b-c (Where it is handled as
a-b, then b-c), that also changed the behaviour of -a-z from -a then -
z to -a then a-z

modified line is 23, from end = s1[++i]; to queue = end = s1[++i];

Again, any comments would be much appreciated!

#include <stdio.h>

/*expands shorthand notations like a-z in the string s1 into the
equivalent
complete list abc...xyz in s2. Allow for letters of either case and
digits,
and be prepared to handle cases like a-b-c and a-z0-9 and -a-z.
Arrange that
a leading or trailing - is taken literally.*/
void expand(char s1[], char s2[])
{
int i, j, queue, start, end;
j = queue = start = end = 0;
for(i = 0; s1[i] != '\0'; i++)
{
if(s1[i] == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1[i]; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1[i]; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1[i];
}
}
}

main()
{
char str1[] = "a-zA-Z";
char str3[] = "-a-zA-Z";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand(str3, str2);
printf("Input: %s\n", str1);
printf("Output: %s\n", str2);
system("pause") ;
}

[Wade Ward]

"Jason" <ra********@gma il.comwrote in message
news:11******** **************@ y5g2000hsa.goog legroups.com...

Slightly modified to handle the case of a-b-c (Where it is handled as
a-b, then b-c), that also changed the behaviour of -a-z from -a then -
z to -a then a-z

modified line is 23, from end = s1[++i]; to queue = end = s1[++i];

Again, any comments would be much appreciated!

#include <stdio.h>

/*expands shorthand notations like a-z in the string s1 into the
equivalent
complete list abc...xyz in s2. Allow for letters of either case and
digits,
and be prepared to handle cases like a-b-c and a-z0-9 and -a-z.
Arrange that
a leading or trailing - is taken literally.*/
void expand(char s1[], char s2[])
{
int i, j, queue, start, end;
j = queue = start = end = 0;
for(i = 0; s1[i] != '\0'; i++)
{
if(s1[i] == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1[i]; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1[i]; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1[i];
}
}
}

The solution I see uses a different control structure that obviates many of
the variables you use. With a while statement that begins while ((c =
s1[i++] = '\0') ..., one begins by fetching a character from s1. Then check
the next character and decide whether to expand the short hand or copy the
character. The C Answer Book is worth its weight in...paper.
--
WW

[Richard Heathfield]

Jason said:

hello everyone! recently i've been working through K&R's The C
programming language, and i just finished exercise 3.3, since my
solution is shorter than the "suggested solution" online, i want to
see if mine is correct to spec.

It doesn't appear to be. It seems not to handle literals properly.

<snip>

>
Any comments would be much appreciated!

Well, let's let the compiler do that.

foo.c:11: warning: no previous prototype for `expand'
foo.c:43: warning: return-type defaults to `int'
foo.c:43: warning: function declaration isn't a prototype
foo.c: In function `main':
foo.c:55: warning: implicit declaration of function `system'
foo.c:56: warning: control reaches end of non-void function

By the way your driver is broken. It claims that str1 is the input,
whereas in fact str3 is the string given to expand().

I took the opportunity to rewrite your driver to take its input from
argv[1] instead.

I gave it this input: TestAj-mTestBb-f
I expected this output: TestAjklmTestBb cdef
I actually got this: jklmbcdef
sh: pause: command not found

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.

[Jason Wang]

On Apr 29, 1:52 am, Richard Heathfield <[e-mail].i nvalidwrote:

Any comments would be much appreciated!

Well, let's let the compiler do that.

foo.c:11: warning: no previous prototype for `expand'
foo.c:43: warning: return-type defaults to `int'
foo.c:43: warning: function declaration isn't a prototype
foo.c: In function `main':
foo.c:55: warning: implicit declaration of function `system'
foo.c:56: warning: control reaches end of non-void function

By the way your driver is broken. It claims that str1 is the input,
whereas in fact str3 is the string given to expand().

I took the opportunity to rewrite your driver to take its input from
argv[1] instead.

I gave it this input: TestAj-mTestBb-f
I expected this output: TestAjklmTestBb cdef
I actually got this: jklmbcdef
sh: pause: command not found

Well, i corrected the code to take all the literals correctly, only a
few changes, all changes are commented.

I'm not sure how to modify the code to take input from argv[1] (i
assume its from user input)..

I also tried to correct as many warnings as i can, there are a few
more that i haven't corrected yet.

I still think its easier to understand than the solution on this page:
http://users.powernet.co.uk/eton/kandr2/krx303.html , or am i doing
something wrong here..

Thanks alot for any further comments!

#include <stdio.h>
/*expands shorthand notations like a-z in the string s1 into the
equivalent
complete list abc...xyz in s2. Allow for letters of either case and
digits,
and be prepared to handle cases like a-b-c and a-z0-9 and -a-z.
Arrange that
a leading or trailing - is taken literally.*/
void expand(char s1[], char s2[])
{
int i, j, start, end, counter;
char queue;
j = queue = start = end = 0;
for(i = 0; s1[i] != '\0'; i++)
{
if(s1[i] == '-')
{
if(queue != 0){
start = queue;
}else{
s2[j++] = s1[i]; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
s2[j++] = s1[i]; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
/* ++start is to skip over the starting char, as its
already copied */
for(++start; start <= end && start != 0 && end != 0; start+
+)
{
s2[j++] = start;
}
end = 0;
start = 0;
counter = 0;
}
else
{
/* copies over all char, in order to handle literal,
however, that means the start of an expansion would be
copied as
well, that means we have to skip over the starting char
in
any expansion in previous if block. This handles Test and
TestB
in the string "TestAj-mTestBb-f" */
s2[j++] = s1[i];
queue = s1[i];
}
}
}

int main()
{
char str1[] = "a-zA-Z";
char str3[] = "-a-zA-Z";
char str4[] = "TEST1-9A-ZTEST";
char str5[] = "TestAj-mTestBb-f";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand(str5, str2);
printf("Input: %s\n", str5);
printf("Output: %s\n", str2);
system("pause") ;
return 1;
}

[Wade Ward]

"Jason Wang" <ra********@gma il.comwrote in message
news:11******** **************@ p77g2000hsh.goo glegroups.com.. .

On Apr 29, 1:52 am, Richard Heathfield <[e-mail].i nvalidwrote:

Any comments would be much appreciated!

Well, let's let the compiler do that.

foo.c:11: warning: no previous prototype for `expand'
foo.c:43: warning: return-type defaults to `int'
foo.c:43: warning: function declaration isn't a prototype
foo.c: In function `main':
foo.c:55: warning: implicit declaration of function `system'
foo.c:56: warning: control reaches end of non-void function

By the way your driver is broken. It claims that str1 is the input,
whereas in fact str3 is the string given to expand().

I took the opportunity to rewrite your driver to take its input from
argv[1] instead.

I gave it this input: TestAj-mTestBb-f
I expected this output: TestAjklmTestBb cdef
I actually got this: jklmbcdef
sh: pause: command not found

Well, i corrected the code to take all the literals correctly, only a
few changes, all changes are commented.

I'm not sure how to modify the code to take input from argv[1] (i
assume its from user input)..

I also tried to correct as many warnings as i can, there are a few
more that i haven't corrected yet.

I still think its easier to understand than the solution on this page:
http://users.powernet.co.uk/eton/kandr2/krx303.html , or am i doing
something wrong here..

Thanks alot for any further comments!

#include <stdio.h>
/*expands shorthand notations like a-z in the string s1 into the
equivalent
complete list abc...xyz in s2. Allow for letters of either case and
digits,
and be prepared to handle cases like a-b-c and a-z0-9 and -a-z.
Arrange that
a leading or trailing - is taken literally.*/
void expand(char s1[], char s2[])
{
int i, j, start, end, counter;
char queue;
j = queue = start = end = 0;
for(i = 0; s1[i] != '\0'; i++)
{
if(s1[i] == '-')
{
if(queue != 0){
start = queue;
}else{
s2[j++] = s1[i]; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
s2[j++] = s1[i]; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
/* ++start is to skip over the starting char, as its
already copied */
for(++start; start <= end && start != 0 && end != 0; start+
+)
{
s2[j++] = start;
}
end = 0;
start = 0;
counter = 0;
}
else
{
/* copies over all char, in order to handle literal,
however, that means the start of an expansion would be
copied as
well, that means we have to skip over the starting char
in
any expansion in previous if block. This handles Test and
TestB
in the string "TestAj-mTestBb-f" */
s2[j++] = s1[i];
queue = s1[i];
}
}
}

int main()
{
char str1[] = "a-zA-Z";
char str3[] = "-a-zA-Z";
char str4[] = "TEST1-9A-ZTEST";
char str5[] = "TestAj-mTestBb-f";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand(str5, str2);
printf("Input: %s\n", str5);
printf("Output: %s\n", str2);
system("pause") ;
return 1;
}

{
....
while ((c = s1[i++]) != '\0')
if (s1[i] == '-' >= c) {
i++;
while (c < s1[i]) /* expand shorthand */
s2[j++] = c++;
} else
s2[j++] = c; /* copy character */
s2[j] = '\0';
}
I think that the trouble you're having is from having the wrong control
structure. An embedded while loops looks good. That's why you need 8 more
variables than did Axel Schreiner for this task, and why compilers warn.
--
WW

[Jason Wang]

>

{
...
while ((c = s1[i++]) != '\0')
if (s1[i] == '-' >= c) {
i++;
while (c < s1[i]) /* expand shorthand */
s2[j++] = c++;} else

s2[j++] = c; /* copy character */
s2[j] = '\0';}

I think that the trouble you're having is from having the wrong control
structure. An embedded while loops looks good. That's why you need 8 more
variables than did Axel Schreiner for this task, and why compilers warn.

I don't think your program works completely, for example, it doesn't
work if the string entered was a-dA-D (where it should expand to
abcdABCD, but instead it expands to abcd-), i think it skips too many
characters to handle anything more than simple expansions. By the way,
i don't think i even used 8 variables for the program, the compiler
couldn't possibly have complained about alot of variables. A for loop
is just a more specific version of while, i think i made the right
choice (as right as one can be).. I can't find out who Axel Schreiner
is either.

[Wade Ward]

"Jason Wang" <ra********@gma il.comwrote in message
news:11******** **************@ q75g2000hsh.goo glegroups.com.. .

{
...
while ((c = s1[i++]) != '\0')
if (s1[i] == '-' >= c) {
i++;
while (c < s1[i]) /* expand shorthand */
s2[j++] = c++;} else

s2[j++] = c; /* copy character */
s2[j] = '\0';}

I think that the trouble you're having is from having the wrong control
structure. An embedded while loops looks good. That's why you need 8
more
variables than did Axel Schreiner for this task, and why compilers warn.

I don't think your program works completely, for example, it doesn't
work if the string entered was a-dA-D (where it should expand to
abcdABCD, but instead it expands to abcd-), i think it skips too many
characters to handle anything more than simple expansions. By the way,
i don't think i even used 8 variables for the program, the compiler
couldn't possibly have complained about alot of variables. A for loop
is just a more specific version of while, i think i made the right
choice (as right as one can be).. I can't find out who Axel Schreiner
is either.

Were you able to fill in the ellipses and make an executable? As I see no
output that doesn't look like speculation, I'll assume the answer is "no."
I read the statement where you assigned zero to all your vars as a
declaration and set the number of extraneous variables at 8 rather than 2,
but I'll claim that your difficulties still arise from having a control
structure that lends itself more poorly to the problem than does the
embedded while loop.

I don't feel like taking this problem to the next level and writing callers,
but take a look at your counterexample. How is it that a-dA-D loses A?
Does'nt the *next* character, the one after 'd', at a minimum, get copied?
--
Wade Ward

[Jason Wang]

On May 1, 2:53 am, "Wade Ward" <inva...@invali d.netwrote:

Were you able to fill in the ellipses and make an executable? As I see no
output that doesn't look like speculation, I'll assume the answer is "no."
I read the statement where you assigned zero to all your vars as a
declaration and set the number of extraneous variables at 8 rather than 2,
but I'll claim that your difficulties still arise from having a control
structure that lends itself more poorly to the problem than does the
embedded while loop.

I don't feel like taking this problem to the next level and writing callers,
but take a look at your counterexample. How is it that a-dA-D loses A?
Does'nt the *next* character, the one after 'd', at a minimum, get copied?

as a matter of fact, i did write out the program, here it is:

void expand2(char s1[], char s2[])
{
int c, i, j;
i = c = j = 0;
while((c = s1[i++]) && (c != '\0'))
{
if(s1[i++] == '-')
{
while(c < s1[i])
{
s2[j++] = c++;
}
}
else
{
s2[j++] = c;
}
}
}

int main()
{
char str1[] = "a-dA-D";
char str3[] = "-a-zA-Z";
char str4[] = "TEST1-9A-ZTEST";
char str5[] = "TestAj-mTestBb-f";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand2(str1, str2);
printf("Input: %s\n", str1);
printf("Output: %s\n", str2);
system("pause") ;
return 1;
}

You can change around the test cases and see the difference between my
program and yours. Also, you said variables originally, not
declarations, and i really don't think it matters. I do admit, if
somehow you can make your program works without significantly changing
it, it is more elegant than mine.

[Wade Ward]

"Jason Wang" <ra********@gma il.comwrote in message
news:11******** **************@ n59g2000hsh.goo glegroups.com.. .

On May 1, 2:53 am, "Wade Ward" <inva...@invali d.netwrote:

>Were you able to fill in the ellipses and make an executable? As I see
no
output that doesn't look like speculation, I'll assume the answer is
"no."
I read the statement where you assigned zero to all your vars as a
declaration and set the number of extraneous variables at 8 rather than
2,
but I'll claim that your difficulties still arise from having a control
structure that lends itself more poorly to the problem than does the
embedded while loop.

I don't feel like taking this problem to the next level and writing
callers,
but take a look at your counterexample. How is it that a-dA-D loses A?
Does'nt the *next* character, the one after 'd', at a minimum, get
copied?

as a matter of fact, i did write out the program, here it is:

void expand2(char s1[], char s2[])
{
int c, i, j;
i = c = j = 0;
while((c = s1[i++]) && (c != '\0'))
{
if(s1[i++] == '-')
{
while(c < s1[i])
{
s2[j++] = c++;
}
}
else
{
s2[j++] = c;
}
}
}

int main()
{
char str1[] = "a-dA-D";
char str3[] = "-a-zA-Z";
char str4[] = "TEST1-9A-ZTEST";
char str5[] = "TestAj-mTestBb-f";
char str2[1000];
int i;
for(i = 0; i<1000; i++)
{
str2[i] = 0;
}
expand2(str1, str2);
printf("Input: %s\n", str1);
printf("Output: %s\n", str2);
system("pause") ;
return 1;
}

You can change around the test cases and see the difference between my
program and yours. Also, you said variables originally, not
declarations, and i really don't think it matters. I do admit, if
somehow you can make your program works without significantly changing
it, it is more elegant than mine.

You snipped away the source snippet and cut a little too close to the bone:

>while ((c = s1[i++]) != '\0')
if (s1[i] == '-' >= c) {

----- > i++;

>while (c < s1[i]) /* expand shorthand */
s2[j++] = c++;} else

s2[j++] = c; /* copy character */

---- > s2[j] = '\0';}
You removed two of the statements in Axel's finished product. Declare c as
a char too, and see how it goes. Gotta run
--
ww