2

I am creating this program as part of an assignment for college. The objective is to copy char* slogan = "Comp10120 is my favourite module"; to a new string while removing consonants and capitalising all letters. This is my code:

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

void printStrings();

char *slogan = "Comp10120 is my favourite module";
char *p = slogan;
char *slogan_copy = NULL;

int main ()
{   
    //Get size of original string
    int slogan_size = 0;
    while (*p++ != '\0')
        slogan_size++;

    // Dynamically allocate memory to copy of string
    slogan_copy = (char*) malloc ((slogan_size+1) * sizeof(char));
    //Place string terminator at end of copy string
    slogan_copy[slogan_size] = '\0';

    //Reset p pointer to start of string
    p = slogan;
    int offset = 0;

    while (*p != '\0')
    {
        //If the current element in the string is a consonant,
        //or as defined in the if statement,
        //if p is not a vowel and is between a and z or A and Z:
        if ((!(*p == 'a' || *p == 'e' || *p == 'i' || *p == 'o' || *p == 'u')) 
            && (((*p > 'a') && (*p < 'z')) || ((*p > 'A') && (*p < 'Z'))))
            p++;
        else
            //Copy element to slogan_copy and capitalise
            slogan_copy[offset++] = *p++;
            slogan_copy[offset] = toupper(slogan_copy[offset]);
    }

    //Place string terminator after last element copied.
    slogan_copy[offset] = '\0';

    printStrings();

    return 0;
}

void printStrings ()
{
    printf("Origianl String: %s\n",*slogan);
    printf("Modified String: %s",*slogan_copy);
}

When I try to execute, I get the error

initializer element is not constant
 char *p = slogan;
           ^~~~~~

I am assuming that it is because I am trying to perform operations on slogan as if it was just a regular array of characters, and not a pointer to a string. However, I don't know how to fix this error.

In addition to this, I tried changing char*slogan = "Comp10120 is my favourite module"; to char slogan[] = "Comp10120 is my favourite module"; to see if it would work, out of curiosity. It complies, but crashes upon execution. Any ideas as to how I could modify my code for it to work?

4
  • printf("Origianl String: %s\n",*slogan); You are passing a char when the function expects a pointer. Should be: printf("Origianl String: %s\n", slogan); Commented Mar 8, 2018 at 13:53
  • And put char *p = slogan; at the beginning of main, then it will compile. But anyway your code is terribly complicated. And your abuse us global variables is questionable. Commented Mar 8, 2018 at 13:53
  • 1
    Also, char *slogan = .... should be const char *slogan = ...., which means char *p = slogan; should be const char *p = slogan;. Commented Mar 8, 2018 at 14:03
  • @MichaelWalz If you have time, could you suggest a way to make it a little less complicated? I'm still trying to learn. Commented Mar 8, 2018 at 14:51

4 Answers 4

1

you have quite a lot of mistakes in your program. do consider your use of global variables and consider using const where it is needed, However it is a preety good starting point so I have tested your program and it seems to work with 4 simple corrections:

1.remove p initialazation in the global env

8: //char *p = slogan;
9: char *p;
  1. set p within main block

    int main () { p = slogan; ... }

  2. remove the astrix from the slogan in your printf statments it is already a pointer to a char array

    printf("Origianl String: %s\n",slogan); printf("Modified String: %s",slogan_copy);

hope this helps

Sign up to request clarification or add additional context in comments.

2 Comments

I doubt that your program works. Does it print capital letters? There is a logic flaw which prevents that.
It doesn't answer the original question but it gives a better starting point to debug the logic
0

A few improvements are needed.

1) In the function printf format %s expects pointer to the buffer. There is no need to dereference slogan or slogan_copy.

2)

slogan_copy[offset++] = *p++;
slogan_copy[offset] = toupper(slogan_copy[offset]);

The above will not work. It will make the next character upper not the current.

3) In C language there is no need to cast malloc.

4) Global variables should be avoided at all cost. The break encapsulation. Pass variables as a parameters, you will get greater flexibility.

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

void printStrings (const char *format, const char *s);

int main (void)
{   
    char *slogan = "Comp10120 is my favourite module";
    char *p = slogan;
    char *slogan_copy;
    int offset = 0;
    int slogan_size = 0;

    //Get size of original string

    while (*p++ != '\0')
        slogan_size++;

    // Dynamically allocate memory to copy of string

    slogan_copy = malloc ((slogan_size+1) * sizeof(char));

    //Place string terminator at end of copy string
    slogan_copy[slogan_size] = '\0';

    //Reset p pointer to start of string
    p = slogan;

    while (*p != '\0')
    {
        //If the current element in the string is a consonant,
        //or as defined in the if statement,
        //if p is not a vowel and is between a and z or A and Z:
        if ((!(*p == 'a' || *p == 'e' || *p == 'i' || *p == 'o' || *p == 'u')) 
            && (((*p > 'a') && (*p < 'z')) || ((*p > 'A') && (*p < 'Z'))))
            p++;
        else{
            //Copy element to slogan_copy and capitalise
            slogan_copy[offset] = *p;
            slogan_copy[offset] = toupper(slogan_copy[offset]);

            *p++;
            offset++;
        }
    }

    //Place string terminator after last element copied.
    slogan_copy[offset] = '\0';

    printStrings("Origianl String: %s\n", slogan);
    printStrings("Modified String: %s\n", slogan_copy);

    return 0;
}

void printStrings (const char *format, const char *s)
{
    printf(format,s);
}

Output:

Origianl String: Comp10120 is my favourite module                                                                                            
Modified String: O10120 I  AOUIE OUE  

Comments

0

As per your request in a comment here is a simplified and correct version:

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

int main()
{
  const char *slogan = "Comp10120 is my favourite module";

  // Dynamically allocate memory to copy of string
  char *slogan_copy = malloc((strlen(slogan) + 1) * sizeof(char));

  //Reset p pointer to start of string
  const char *p = slogan;
  int offset = 0;

  while (*p != '\0')
  {
    //If the current element in the string is a consonant,
    //or as defined in the if statement,
    //if p is not a vowel and is between a and z or A and Z:
    char c = toupper(*p++);
    if (!isalpha(c) || c == 'A' || c == 'E' || c == 'I' || c == 'O' || c == 'U' || )
      slogan_copy[offset++] = c;
  }

  //Place string terminator after last element copied.
  slogan_copy[offset] = '\0';

  printf("Origianl String: %s\n", slogan);
  printf("Modified String: %s\n", slogan_copy);

  return 0;
}

Output:

Origianl String: Comp10120 is my favourite module
Modified String: O10120 I  AOUIE OUE

Comments

0

Your code is not indented correctly: the else branch has 2 statements but they are not wrapped inside a block with { and }, so only the first statement in executed conditionally and the second is always executed, causing unexpected behavior regarding the uppercasing feature.

Furthermore, the uppercasing is not applied to the correct offset as offset is incremented too early, and uppercase vowels would be removed too.

A sane rule for coding style is to always use braces for all compound statements but the simplest ones. Rewrite the test this way:

    int c = toupper((unsigned char)*p++);
    if (!isalpha(c) || c == 'A' || c == 'E' || c == 'I' || c == 'O' || c == 'U') {
        //Copy and capitalise element to slogan_copy
        slogan_copy[offset++] = c;
    }

There are other problems in the code, eg: passing incorrect data for the printf arguments, and using global variables for no good reason.

Here is an improved version:

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

int main() {   
    const char *slogan = "Comp10120 is my favourite module";
    char *slogan_copy = NULL;

    //Get size of original string
    int slogan_size = 0;
    while (slogan[slogan_size] != '\0')
        slogan_size++;

    // Dynamically allocate memory to copy of string
    slogan_copy = malloc(slogan_size + 1);

    //Reset p pointer to start of string
    const char *p = slogan;
    int offset = 0;

    while (*p != '\0') {
        int c = toupper((unsigned char)*p++);
        if (!isalpha(c) || c == 'A' || c == 'E' || c == 'I' || c == 'O' || c == 'U') {
            //Copy and capitalise element to slogan_copy
            slogan_copy[offset++] = c;
        }
    }

    //Place string terminator after last element copied.
    slogan_copy[offset] = '\0';

    printf("Original string: %s\n", slogan);
    printf("Modified string: %s\n", slogan_copy);

    return 0;
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.