CODEWITHSUNDEEP
c
user379888
user379888

Reputation:

preprocessor directive not working

I made a program which removes spaces and makes a string upper case by preprocessor directives .its not changing to uppercase

#include <stdio.h>
#include <conio.h>
# define TOUPPER(x) (x-32)
void main(void)
{
    int i,j;
    char str[100],*p;
    clrscr();
    printf("Enter the string:\n");
    gets(str);
    for(i=0;    ;i++)
    {
        if(str[i]=='\0')
        break;
        if(str[i]==' ')
        {
            for(j=i;    ;j++)
            {
             str[j]=str[j+1];
             if(str[j]=='\0')
             break;
            }
         }
         if(str[i]<='a'||str[i]>='z')
         {
            *p=str[i];
            TOUPPER('p');

         }


        }
        puts(str);
    getch();
}

Upvotes: 0

Views: 427

Answers (6)

Roland Illig
Roland Illig

Reputation: 41617

Don't believe that using preprocessor macros is always faster than using the functions from <ctype.h>. It is highly probable that these functions are in fact implemented as preprocessor macros themselves. Just write code that is easy to understand, and only "optimize" it if it is necessary.

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

static void
str_toupper(char *str)
{
    char *p;

    for (p = str; *p != '\0'; p++) {
        if (isspace((unsigned char) *p)) {
            memmove(p + 0, p + 1, strlen(p) - 1);
        }
        *p = toupper((unsigned char) *p);
    }
}

int main(void)
{
    char str[100];

    clrscr();
    printf("Enter the string:\n");
    if (fgets(str, sizeof str, stdin) == NULL)
        return 0;

    str_toupper(str);

    printf("%s\n", str);
    getch();
    return 0;
}

Upvotes: 1

Jason S
Jason S

Reputation: 189626

This isn't what you're asking, but please note you have a serious pointer error in your program:

char str[100],*p;

  ...

if(str[i]<='a'||str[i]>='z')
     {
        *p=str[i];
        TOUPPER('p');

     }

The pointer p is uninitialized (can be pointing to anywhere in memory), and you are assigning the character str[i] to that pointer's memory location. p might be equal to 0x00000000 or 0xCDCDCDCD or something else, but in any case you are writing str[i] to that location without initializing p to an appropriate address.

If you feel you must use pointers, you could do it this way (still doesn't fix the range issue that others mentioned, the <= and >= are backwards):

if (str[i]<='a'||str[i]>='z')
{
   p=&str[i]; // p now points to the address of str[i]
   *p = TOUPPER(*p); // p = the address, *p = the contents of that address
}

Upvotes: 1

thequark
thequark

Reputation: 756

Problem is wrong range condition, Macro call with constant argument and you are not assigning the value back.

if(str[i]<='a'||str[i]>='z')
         {
            *p=str[i];
            TOUPPER('p');

         }

Correction:

if(str[i]>='a'||str[i]<='z')
         {
            str[i] = TOUPPER(str[i]);
         }

Upvotes: 4

Doc Brown
Doc Brown

Reputation: 20044

Perhaps this is what you wanted:

str[i]=TOUPPER(str[i])

(And don't mess around with those pointer p)

Upvotes: 3

Greg D
Greg D

Reputation: 44066

Your TOUPPER('p') does exactly what it should, nothing. You're subtracting 32 from the numeric value of 'p' and then throwing it away. Note that I'm referring to 'p' the character, not p the pointer.

Upvotes: 8

Quonux
Quonux

Reputation: 2991

Its because you do

TOUPPER('p');

and it is defined as

(x-32)

This changes 'p' to upper-case but don't save it.

You need to change you define like this

#define TOUPPER(x) ((*x)=((*x)-32))

just do call it like this:

TOUPPER(p);

Upvotes: 2

Related Questions