concatenating uppercase letters of two separate strings in c

Question

I am doing practice questions for an upcoming exam but cant seem to figure out a question where the objective is:

Write a C program to read two strings s1 and s2. The maximum size of each string is 25. Form two new strings called upper and lower. Lower consists of the lowercase letters of s1 and s2 concatenated while upper contains the concatenation of the uppercase letters of both strings.

eg: Input:

s1: GREENblue
s2: busCAR

Output:

upper: GREENCAR
lower: bluebus

I've been attempting this for a few days but have not been able to completely figure it out.

Here's what I have so far:

#include <stdio.h>
#include <math.h>
#include <string.h>
#include <conio.h>

int main()
{
    int i, j;
    char s1[25];
    char s2[25];
    char upper[25];
    char lower[25];

    printf ("Please enter first string: ");
    scanf ("%s", &s1);
    printf ("Please enter second string: ");
    scanf ("%s", &s2);

    strcat (s1, s2);

    for (i=0; i<strlen(s1); i++)
    {
        if (s1[i]>=65 && s1[i] <= 90)
        {
            upper[i] = s1[i];
        }
        else
        {
            lower[i] = s1[i];
        }
    }
    printf ("Upper: %s\n", upper);
    printf ("Lower: %s\n", lower);
}

`scanf ("%s", &s1);` --> `scanf ("%24s", s1);`, same for `s2` — Sourav Ghosh, Jun 17 '15 at 12:46
any particular reason for the downvote? On both the question and the first answer?! — Pandrei, Jun 17 '15 at 16:05

Sourav Ghosh · Answer 1 · 2015-06-17T13:30:31.297

In your code, in the usage of strcat()

 strcat (s1, s2);

s1 is probably not having enough memory to hold the concatenated string. In that case, it invokes undefined behaviour.

From the man page,

If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs.

As per your requirement

The maximum size of each string is 25

You need a destination buffer of length 25 X 2 (for chars) + 1 (for null).

Also, it indicates, you need s1 and s2 to have 26 elements, each, for accommodating the null.

As per your requirement, there are certain design changes required to your program, as

you need s1 and s2 to have 26 elements, each, for accommodating the null.
upper and lower both must have a size of total of the input size. Think of the scenario if s1 and s2 are both completely UPPERCASE (or lowercase). So, you need upper[51] and lower[51].

@RhysHinkson regarding buffer size? I'll add a line. – Sourav Ghosh Jun 17 '15 at 13:27 — Sourav Ghosh, Jun 17 '15 at 13:27

Vlad from Moscow · Answer 2 · 2015-06-17T13:25:06.837

Here's what i have so far: :)

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

#define N   25

int main(void)
{
    char s1[N];
    char s2[N];
    char *upper, *lower;
    char *p, *q;
    size_t n1, n2;

    printf( "Please enter first string: " );
    fgets( s1, N, stdin );

    printf( "Please enter second string: " );
    fgets( s2, N, stdin );

    n1 = 0; n2 = 0;

    for ( p = s1; *p; ++p )
    {
        if ( isupper( ( unsigned char )*p ) ) ++n1;
        else if ( islower( ( unsigned char )*p ) ) ++n2;
    }

    for ( p = s2; *p; ++p )
    {
        if ( isupper( ( unsigned char )*p ) ) ++n1;
        else if ( islower( ( unsigned char )*p ) ) ++n2;
    }

    upper = NULL; lower = NULL;

    if ( n1 && ( upper = ( char * )malloc( n1 + 1 ) ) )
    {
        q = upper;

        for ( p = s1; *p; ++p )
        {
            if ( isupper( ( unsigned char )*p ) ) *q++ = *p;
        }

        for ( p = s2; *p; ++p )
        {
            if ( isupper( ( unsigned char )*p ) ) *q++ = *p;
        }

        *q = '\0';
    }       

    if ( n2 && ( lower = ( char * )malloc( n1 + 1 ) ) )
    {
        q = lower;

        for ( p = s1; *p; ++p )
        {
            if ( islower( ( unsigned char )*p ) ) *q++ = *p;
        }

        for ( p = s2; *p; ++p )
        {
            if ( islower( ( unsigned char )*p ) ) *q++ = *p;
        }

        *q = '\0';
    }       

    if ( upper ) printf( "Upper: %s\n", upper );
    if ( lower ) printf( "Lower: %s\n", lower );

    free( upper );
    free( lower );

    return 0;
}

The program output might look like

Please enter first string: GREENblue
Please enter second string: busCAR
Upper: GREENCAR
Lower: bluebus

If your compiler supports Variable Length Arrays then you could use VLA(s) instead of the dynamically allocated arrays.

Here is a program that uses VLA

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

#define N   25

int main(void)
{
    char s1[N];
    char s2[N];

    printf( "Please enter first string: " );
    fgets( s1, N, stdin );

    printf( "Please enter second string: " );
    fgets( s2, N, stdin );

    size_t n1 = 0, n2 = 0;

    for ( const char *p = s1; *p; ++p )
    {
        if ( isupper( ( unsigned char )*p ) ) ++n1;
        else if ( islower( ( unsigned char )*p ) ) ++n2;
    }

    for ( const char *p = s2; *p; ++p )
    {
        if ( isupper( ( unsigned char )*p ) ) ++n1;
        else if ( islower( ( unsigned char )*p ) ) ++n2;
    }


    char upper[n1 + 1];
    char lower[n2 + 1];

    if ( n1 )
    {
        char *q = upper;

        for ( const char *p = s1; *p; ++p )
        {
            if ( isupper( ( unsigned char )*p ) ) *q++ = *p;
        }

        for ( const char *p = s2; *p; ++p )
        {
            if ( isupper( ( unsigned char )*p ) ) *q++ = *p;
        }

        *q = '\0';
    }       

    if ( n2 )
    {
        char *q = lower;

        for ( const char *p = s1; *p; ++p )
        {
            if ( islower( ( unsigned char )*p ) ) *q++ = *p;
        }

        for ( const char *p = s2; *p; ++p )
        {
            if ( islower( ( unsigned char )*p ) ) *q++ = *p;
        }

        *q = '\0';
    }       

    if ( n1 ) printf( "Upper: %s\n", upper );
    if ( n2 ) printf( "Lower: %s\n", lower );


    return 0;
}

score 0 · Answer 3 · answered Jun 17 '15 at 15:42

0

You still have a problem: what happens if you enter a string that is longer than 23 characters? Your program will crash.

If the maximum size is to be 25 characters, your arrays have to be dimensioned 26, not 25, to account for the terminating NULL.

Then you have to set the s1[N-1]='\0' and s2[N-1] = '\n' for safety. If you enter something too long for the first string, the second string will end up being the residual characters, ignoring anything you entered after the prompt for the second string.

answered Jun 17 '15 at 15:42

FredK

4,094
1
9
11

Detail: `scanf("%s", &s1);` (which should be `scanf("%s", s1);` or better `scanf("%24s", s1);`) will not save a `'\n'` in `s1`. Code has trouble if user input exceeds 24 non-white-space characters, not 23. – chux - Reinstate Monica Jun 17 '15 at 17:13

score -1 · Accepted Answer · edited Jun 17 '15 at 17:05

I think you got the general idea but missed a few details which are important:

don't use strcat to concatenate the two strings. You already have each one of them in their own buffer plus, if you are going to do this you have better make sure you have enough memory for the larger resulting string.
if (s1[i]>=65 && s1[i] <= 90) this condition is good to check for upper case letters but it's negation leaves you with everything else, which includes lower case letters, numbers, etc. So it better to have 3 cases (uppercase, lowercase, everything else) just to be safe).

if you are going to print the content of the upper/lower arrays, make sure you append the '\0' character so they are interpreted as strings by your print function.

j=0; //iterator for lower
k=0; //iterator for upper
for (i=0; i<strlen(s1); i++)
{
    if (s1[i]>='A' && s1[i] <= 'Z') // a bit more clear
    {
        upper[k++] = s1[i];
    }
    else
    {
        if (s1[i]>='a' && s1[i] <= 'z')
        {
             lower[j++] = s1[i];
        }
        else
        {
            //print something or just exit...
        }
     }
}

for (i=0; i<strlen(s2); i++)
{
    if (s2[i]>='A' && s2[i] <= 'Z') // a bit more clear
    {
        upper[k++] = s2[i];
    }
    else
    {
        if (s2[i]>='a' && s2[i] <= 'z')
        {
            lower[j++] = s2[i];
        }
        else
        {
             //print something or just exit...
        }
    }
}
//add the string terminator character
lower[j] = '\0';
upper[k] = '\0';

//print the result
printf ("Upper: %s\n", upper);
printf ("Lower: %s\n", lower);

This should do the job.

yes :) this worked and i understand it thanks very much all of you for the assistance i'll keep all your pointers in mind...thanks again — TriniProgrammer, Jun 17 '15 at 13:53
@chux actually that is not true, the compiler will precompute the value of strlen(s1)/strlen(s2) even before if gets to the for loops as part of the optimization process. So it's actually the same as doing it explicitly in the code. — Pandrei, Jun 18 '15 at 08:14
@Pandrei A compiler may do that optimization only if if "knows" is the `strlen()` we all know and love and not some other `strlen()` that has side effects. Further, that single call, as you comment, can only be employed if the compiler analyzes all possible execution paths in the loop to insure `s1` did not change. This is a tall order that many compilers simply can not meet. See http://stackoverflow.com/questions/2049480/how-many-times-will-strlen-be-called-in-this-for-loop — chux - Reinstate Monica, Jun 18 '15 at 13:46

concatenating uppercase letters of two separate strings in c

4 Answers4