2
#include <stdio.h>
char q[50];
int doo(int p, int n, char* s) {
  int i = 0, j = 0;
  while (s[i] != '\0') {
    if ((i < p) && (i > (p + n))) {
      q[j] = s[i];
      ++j;
    }
    i++;
  }
  puts(p);
}
int main() {
  char s[50];
  int n, p;
  printf("<--program by gautam-->\n");
  printf("enter the string-->");
  gets(s);
  printf("\nenter the numbers of character to be deleted and position-->");
  scanf("%d%d", &n, &p);
  --p;
  doo(p, n, s);
  return 0;
}

The task is to delete certain elements of a string by asking the user the position and number of elements to delete. I'm trying to copy all elements except those whose position is provided by user, but I'm getting no output at all.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • `puts(p)` ? What are you trying to do here. `p` is an `int`. – WedaPashi Jun 09 '21 at 17:12
  • 1
    `q` will overflow if someone enters more than 49 characters. – WedaPashi Jun 09 '21 at 17:14
  • Already a solution provided here, which covers almost everything. Also, as different design approach you could look at if `strncat()` and `strncpy()` work for you here. I am certain that these would make code a lot more readable. – WedaPashi Jun 09 '21 at 17:32

1 Answers1

2

The fundamental error in your code is that you are using the && operator in the test inside your while loop, whereas you should be using the || operator: if either of the conditions is true, then add the character to the output string.

Also, your doo function is declared as returning an int but it doesn't return anything: I fixed this in the code below by returning j (the count of characters copied) but, as you never use that, you may want to redeclare the function as void, instead.

You are also attempting to print p with the puts function, where you most likely want to print q (I can put this one down to a typo).

Lastly: Never use the gets function! Why is the gets function so dangerous that it should not be used? Use fgets(), instead - it's much safer, as it will never read in more characters than you tell it to, so the buffer won't overflow (if you specify the correct size).

Here's a 'fixed' version of your code, with comments added where I've made changes:

#include <stdio.h>
char q[50];

int doo(int p, int n, char* s)
{
    int i = 0, j = 0;
    while (s[i] != '\0') {
        if ((i < p) || (i > (p + n))) { // Need OR not AND here
            q[j] = s[i];
            ++j;
        }
        i++;
    }
    q[j] = '\0'; // Add nul terminator (don't need if only calling once, but best to have it anyway)
    puts(q); // Should be printing "q" (result string) NOT "p" (position).
    return j; // MUST return something - here, the count of characters
}

int main()
{
    char s[50];
    int n, p;
    printf("<--program by gautam-->\n");
    printf("enter the string-->");
    fgets(s, 50, stdin); // NEVER use gets - it's been removed from the language!
    printf("\nenter the numbers of character to be deleted and position-->");
    scanf("%d%d", &n, &p);
    --p;
    doo(p, n, s);
    return 0;
}
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • You beat me by a few seconds :-) Also, can you add something about issues with size of `q`. – WedaPashi Jun 09 '21 at 17:33
  • @WedaPashi I think the `50` argument to `fgets` will take care of that. But you're right, maybe I should add a note of explanation ... – Adrian Mole Jun 09 '21 at 17:34
  • Oh, I didn't see that. Sorry! :-) – WedaPashi Jun 09 '21 at 17:35
  • hey thanks for help, can you tell me one thing if i declare string q inside "doo" function instead declaring it globally, how can i return it to main, so that i can print it from main. – Gautam Bisht Jun 09 '21 at 17:37
  • @GautamBisht If you want to do that, then you will need to `malloc` the destination buffer and return that pointer (which you must `free()` later on). Or, you *could* use a `static char q[50];`, but that would keep reusing/overwriting the buffer each time you call the function. – Adrian Mole Jun 09 '21 at 17:39