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

J

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 != '\0'; i++)
{
if(s1 == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1; /* handles trailing '-' */
}else{
end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1;
}
}
}

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


Any comments would be much appreciated!
 
J

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 != '\0'; i++)
{
if(s1 == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1;
}
}
}

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

Wade Ward

Jason said:
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 != '\0'; i++)
{
if(s1 == '-')
{
if(queue != 0){
start = queue;
}else{
start = s1; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
end = s1; /* handles trailing '-' */
}else{
queue = end = s1[++i];
}
for(; start <= end; start++)
{
s2[j++] = start;
}
end = 0;
start = 0;
}
else
{
queue = s1;
}
}
}
Click to expand...

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.
 
R

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.
Click to expand...

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

Any comments would be much appreciated!
Click to expand...

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: TestAjklmTestBbcdef
I actually got this: jklmbcdef
sh: pause: command not found
 
J

Jason Wang

Any comments would be much appreciated!
Click to expand...

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: TestAjklmTestBbcdef
I actually got this: jklmbcdef
sh: pause: command not found
Click to expand...

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 != '\0'; i++)
{
if(s1 == '-')
{
if(queue != 0){
start = queue;
}else{
s2[j++] = s1; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
s2[j++] = s1; /* 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;
queue = s1;
}
}
}

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 = 0;
}
expand(str5, str2);
printf("Input: %s\n", str5);
printf("Output: %s\n", str2);
system("pause");
return 1;
}
 
W

Wade Ward

Jason Wang said:
Any comments would be much appreciated!
Click to expand...

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: TestAjklmTestBbcdef
I actually got this: jklmbcdef
sh: pause: command not found
Click to expand...

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 != '\0'; i++)
{
if(s1 == '-')
{
if(queue != 0){
start = queue;
}else{
s2[j++] = s1; /* handles leading '-' */
}
if(s1[i+1] == '\0'){
s2[j++] = s1; /* 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;
queue = s1;
}
}
}

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 = 0;
}
expand(str5, str2);
printf("Input: %s\n", str5);
printf("Output: %s\n", str2);
system("pause");
return 1;
}
Click to expand...

{
....
while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) {
i++;
while (c < s1) /* 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.
 
J

Jason Wang

{
...
while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) {
i++;
while (c < s1) /* 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.
Click to expand...


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.
 
W

Wade Ward

Jason Wang said:
{
...
while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) {
i++;
while (c < s1) /* 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.
Click to expand...


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.
Click to expand...

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?
 
J

Jason Wang

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?
Click to expand...

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)
{
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 = 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.
 
W

Wade Ward

Jason Wang said:
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?
Click to expand...

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)
{
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 = 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.
Click to expand...

You snipped away the source snippet and cut a little too close to the bone:
while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) { ----- >> i++;
while (c < s1) /* expand shorthand */
s2[j++] = c++;} else

s2[j++] = c; /* copy character */
Click to expand...
Click to expand...

---- >> 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
 
J

Jason Wang

You snipped away the source snippet and cut a little too close to the bone:>> while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) { ----- >> i++;
while (c < s1) /* expand shorthand */
s2[j++] = c++;} else
s2[j++] = c; /* copy character */
Click to expand...
Click to expand...


---- >> 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
Click to expand...


First of all, the assumption is that s1 has '\0', so it will
automatically be copied from s1.
Secondly, int is the same as char for all practical purposes here,
since char can be represented at int.
also, this statement: while ((c = s1[i++]) != '\0') is equviliant to
my while((c = s1[i++]) && (c != '\0')) to the best of my
understandings, at least i modified it so that it makes sense to me.
Just to make sure, i modified the statement to what you've wrote, it
simply outputs smilie faces now. Makes even less sense for some
reason.

Please, if you really want to help, at least make sure the program you
post works to spec, i don't want to sound ungrateful, but this
discussion is not contributing to my knowledge.
 
W

Wade Ward

Jason Wang said:
You snipped away the source snippet and cut a little too close to the
bone:>> while ((c = s1[i++]) != '\0')
if (s1 == '-' >= c) { ----- >> i++;
while (c < s1) /* expand shorthand */
s2[j++] = c++;} else
Click to expand...

s2[j++] = c; /* copy character */
Click to expand...

---- >> 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
Click to expand...


First of all, the assumption is that s1 has '\0', so it will
automatically be copied from s1.
Secondly, int is the same as char for all practical purposes here,
since char can be represented at int.
also, this statement: while ((c = s1[i++]) != '\0') is equviliant to
my while((c = s1[i++]) && (c != '\0')) to the best of my
understandings, at least i modified it so that it makes sense to me.
Just to make sure, i modified the statement to what you've wrote, it
simply outputs smilie faces now. Makes even less sense for some
reason.

Please, if you really want to help, at least make sure the program you
post works to spec, i don't want to sound ungrateful, but this
discussion is not contributing to my knowledge.
Click to expand...

You lack the ability to take published source off of usenet. Have fun in
c.l.c.
 
P

Peter 'Shaggy' Haywood

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.
Click to expand...

I think you've got that wrong. I think the intention is to take a
leading or trailing '-' as simply '-'. A '-' between two characters
expands to all characters between the two, and all other characters are
copied intact. So "-a-z" should expand to "-abcdefghijklmnopqrstuvwxyz".
Here's my hacked-up-in-minutes solution:

#include <stdio.h>

#define BIG_ENOUGH_I_HOPE 50

void expand(const char *s1, char *s2)
{
char lastchar;

/* Copy leading '-'. */
if('-' == *s1)
{
*s2++ = *s1++;
lastchar = '-';
}

/* Expand other characters. */
while(*s1)
{
if('-' == *s1)
{
s1++;

if('\0' == *s1)
{
/* Copy trailing '-'. */
*s2++ = '-';
break;
}
else
{
/* Expand range between last character and next
character (including next character -
last character has already been copied). */
char ch;

if(*s1 >= lastchar)
{
/* Handle range in ascending order. */
for(ch = lastchar + 1; ch < *s1; ch++)
{
*s2++ = ch;
}
}
else
{
/* Handle range in descending order. */
for(ch = lastchar - 1; ch > *s1; ch--)
{
*s2++ = ch;
}
}

lastchar = *s1;
}
}
else
{
/* Copy literal character. */
lastchar = *s1;
*s2++ = *s1++;
}
}

*s2 = '\0';
}

int main(void)
{
const char *s1 = "-a-d-fA-D0-87-3fdg-";
char s2[BIG_ENOUGH_I_HOPE];

expand(s1, s2);
printf("s1 = \"%s\"\ns2 = \"%s\"\n", s1, s2);

return 0;
}

--
Dig the sig!



----------- Peter 'Shaggy' Haywood ------------

Ain't I'm a dawg!!
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

C Programming Language 2nd Ed, Exercise 1.9 and 1.12, Solution suggestion. 45
K&R2, exercise 5-4, strend(s,t) 15
Small Correction to K&R Exercise 1-22 Solution on CLC-Wiki 8
Errata for The C Programming Language, Second Edition, by Brian Kernighanand Dennis Ritchie 4
[Trolling] assembly vs C language 6
Why C Is Not My Favourite Programming Language 132
error in mystrcat 5
In the Matter of Herb Schildt: a Detailed Analysis of "C: TheComplete Nonsense" 109

Members online

No members online now.
Total: 1,159 (members: 1, guests: 1,158)
Robots: 456

Forum statistics

Threads
474,175
Messages
2,570,941
Members
47,487
Latest member
IKZChelsea

Latest Threads

Top