trjhtr

I have an exercise in my ANSI C book:

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.

I have attempted to solve it and here's my code:

#include <stdio.h>
#include <string.h>
#define MAXLEN 100

void expand(char * s1, char * s2);

int main(void) 
 char s1 = "a-b-c";
 char s2[MAXLEN];
 expand(s1, s2);
 printf("%sn", s2);
 return 0;


void expand(char * s1, char * s2) 
 char prev = '';
 char mid = '';
 char next = '';
 if (s1[0] == '-') 
 *s2 = *s1;
 s1++, s2++;
 
 while (*s1 != '') 
 if (next == '' && mid == '-')
 *s2++ = mid;
 else if (mid == '-') 
 if (prev < next)
 for (char i = prev; i <= next; i++, s2++)
 *s2 = i;
 else
 for (char i = prev; i >= next; i--, s2++)
 *s2 = i;
 
 prev = *s1;
 s1++;
 mid = *s1;
 if (*s1 != '')
 next = *(s1 + 1);
 
 *s2 = *s1;

What do you think about this solution? Does it solve the exercise? Is it a good way to solve it?

#include <string.h>

We don't use any of the definitions of this header, so it can safely be omitted.

#define MAXLEN 100

Why 100? More importantly, what happens when we reach this maximum length? We probably need to add some code to stop producing output when it's reached. The provided interface expand(s1,s2) doesn't allow us to tell the function how much space is available to write into s2, so that's probably out of scope for the exercise, but consider implementing expand(s1,s2,max_len) as a further exercise (and a good one, that will be useful in your future C coding).

What should the output be for a-b-c? Your program produces abbc, but I don't know if that's expected. At least add a comment to document your assumptions. Other possible interpretations include abc and ab-c, so explain why you've chosen abbc for this input.

Another problematic test case: --5.

Use const for the input string. If we also return the start of the output, we can use the function more naturally:

char *expand(const char *s1, char *s2) 
 char *const s2_start = s2;

 /*...*/

 return s2_start;


int main(void) 
 char s2[MAXLEN];
 puts(expand("--5", s2));

Note that I've used puts() as a simpler alternative to sprintf("%sn", ).

Your indexing is odd and inconsistent. I see s1[0] mixed in with *s, and *(s1 + 1) in place of the simpler s1[1]. You can also make use of the value of increment expressions, like this:

if (*s1 == '-') 
 *s2++ = *s1++;

 char i = prev;
 if (prev < next)
 while (i <= next)
 *s2++ = i++;
 else
 while (i >= next)
 *s2++ = i--;

Improved code

I've completely re-written, observing the above changes:

#include <stdio.h>

char *expand(const char *s1, char *s2, int max_len) 
 char *const s2_start = s2;
 char *const s2_end = s2_start + max_len - 1;

 while (*s1 && s2 < s2_end) 
 if (*s1 == '-' && s2 != s2_start && s1[1]) 
 /* spotted hyphen not at beginning or end */
 char first = s1[-1];
 char last = *++s1;
 /* produce a range first..last,
 exclusive (we've already written first) */
 if (first == last) 
 ++s1;
 else if (first < last) 
 /* ascending */
 for (char i = ++first; i < last && s2 < s2_end; ++i)
 *s2++ = i;
 else 
 /* descending */
 for (char i = --first; i > last && s2 < s2_end; --i)
 *s2++ = i;
 
 else 
 /* copy to output */
 *s2++ = *s1++;
 
 
 *s2 = '';

 return s2_start;



#define MAXLEN 100
int main(void) 
 char s2[MAXLEN];
 puts(expand("a-z-a-z-a", s2, MAXLEN));

void expand(char * s1, char * s2);

Although the placement of * (int * a or int* a or int *a), I would stick to one and only one throughout a codebase. Later on you have *s2 = *s1 so it's a good idea to use that style in the function signature as well.