1

The problem: After the convert_tolower(words) function is completed I want to add a new word in the words array( if the words array has less than 5 words)..But I am getting either errors or unexpected results(e.g some weird characters being printed)...What i thought is shifting the elements of the words array and then work with pointers because I am dealing with strings.But I am having quite some trouble achieving that..Probably the problem is in lines 35-37

How I want the program to behave:

  1. Get 5 words(strings) at most from user input
  2. Take these strings and place them in an array words
  3. Convert the elements of the array to lowercase letters
  4. After the above,ask the user again to enter a new word and pick the position of that word.If the words array already has 5 words then the new word is not added.Else,the new word is added in the position the user chose.(The other words are not deleted,they are just 'shifted').

Also by words[1] I refer to the first word of the words array in its entirety

The code:

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

#define W 5
#define N 10

void convert_tolower(char matrix[W][N]);

int main() {
  int j = 0;
  int i = 0;
  int len = 0;
  char words[W][N] = {{}};
  char test[W][N];
  char endword[N] = "end";
  char newword[N];
  int position;
  while (scanf("%9s", test), strcmp(test, endword)) {
    strcpy(words[i++], test);

    j++;
    len++;
    if (j == W) {
      break;
    }
  }

  convert_tolower(words);

  printf("Add a new word\n");
  scanf("%9s", newword);
  printf("\nPick the position\n");
  scanf("%d",position);
  if (len < W) {
    for (i = 0; i < W-1; i++) {
      strcpy(words[i], words[i + 1]); /*Shift the words */
      words[position] = newword;
    }
  }

  for (i = 0; i < W; i++) {
    printf("%s", words[i]);
    printf("\n");
  }

  printf("End of program");
  return 0;
}

void convert_tolower(char matrix[W][N]) {
  int i;
  int j;
  for (i = 0; i < W; i++) {
    for (j = 0; j < N; j++) {
      matrix[i][j] = tolower(matrix[i][j]);
    }
  }
}
15
  • In addition to describing the problem please also define the intended behaviour of your program. Commented Apr 10, 2022 at 6:37
  • 1
    Please add the part of your minimal reproducible example where you init the pointers in words so that they point to memory which you are allowed to write to. I can't find it in the shown code. Commented Apr 10, 2022 at 6:39
  • 1
    Your use of the pointers in words is strange. You define words as 2D, but use it as 1D. I seem to be missing something. Please describe your understanding of what words is. What data type does the 2D array contain? What datatype do you access in the 1D array? If pointers are involved please describe what they are pointing to and why you think you can write there. If "strings" are used, please discuss maximum lenghts and/or termination. Commented Apr 10, 2022 at 6:41
  • I am not that sure I have to work with pointers though.. Commented Apr 10, 2022 at 6:51
  • 2
    Why are you using a 2d array of char * for words? Either use a 1d array of char *, or a 2d array of char. Commented Apr 10, 2022 at 7:05

2 Answers 2

4

This initialization

char words[W][N] = {{}};

is incorrect in C. If you want to zero initialize the array then just write for example

char words[W][N] = { 0 };

In the condition of the while loop

while (scanf("%9s", test), strcmp(test, endword)) {

there is used the comma operator. Moreover you are using incorrectly the two-dimensional array test instead of a one-dimensional array

It seems you mean

char test[N];

//...

while ( scanf("%9s", test) == 1 && strcmp(test, endword) != 0 ) {

And there are used redundantly too many variables like i, j and len.

The loop could be written simpler like

char test[N];

//...

for ( ; len < W && scanf("%9s", test) == 1 && strcmp(test, endword) != 0; ++len ) 
{
    strcpy(words[len], test);
}

In this call

scanf("%d",position); 

there is a typo. You must to write

scanf("%d", &position); 

Also you should check whether the entered value of position is in the range [0, len].

For example

position = -1;

printf("\nPick the position\n");
scanf("%d", &position); 

if ( len < W && -1 < position && position <= len ) {

Also this for loop

for (i = 0; i < W-1; i++) {
  strcpy(words[i], words[i + 1]); /*Shift the words */
  words[position] = newword;
}

does not make a sense. And moreover this assignment statement

  words[position] = newword;

is invalid. Arrays do not have the assignment operator.

You need to move all strings starting from the specified position to the right. For example

for ( i = len; i != position; --i )
{
    strcpy( words[i], words[i-1] );
}

strcpy( words[position], newword );
++len;

And it seems the function convert_tolower should be called for the result array after inserting a new word. And moreover you need to pass the number of actual words in the array.

convert_tolower(words, len);

The nested loops within the function convert_tolower should look at least the following way

void convert_tolower(char matrix[][N], int n) {
  int i;
  int j;
  for (i = 0; i < n; i++) {
    for (j = 0; matrix[i][j] != '\0'; j++) {
      matrix[i][j] = tolower(( unsigned char )matrix[i][j]);
    }
  }
}
Sign up to request clarification or add additional context in comments.

2 Comments

That must have taken you a long time. You have more patience than me :-)
Just one small remark: -1 < position is more risky than 0 <= position which would still work if the type of position was changed to size_t. Also why not test the return value of scanf() in this case?
1

The main problem with your code was initially that you declared char *words[W][N], then tried to insert strings into this 2d array of pointers. Sparse use of organizing functions, and variables with large scopes than necessary made it hard to read. I think the best way to help you is to show you a working minimal implementation. Step 4 is not sufficiently specified. insert currently shift. It is not clear what should happen if you insert at position after empty slots, or if insert a position before empty slots and in particular if there are non-empty slots after said position.

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

#define W 5
#define N 10

void convert(size_t w, size_t n, char list[][n]) {
    for(size_t i = 0; i < w; i++) {
        for(size_t j = 0; j < n; j++) {
            list[i][j] = tolower(list[i][j]);
        }
    }
}

void insert(size_t w, size_t n, char list[][n], size_t pos, char *word) {
    // out out of bounds
    if(pos + 1 > w) return;

    // shift pos through w - 2 pos
    for(size_t i = w - 2; i >= pos; i--) {
        strcpy(list[i + 1], list[i]);
        if(!i) break;
    }
    // insert word at pos
    strcpy(list[pos], word);
}

void print(size_t w, size_t n, char list[][n]) {
    for (size_t i = 0; i < w; i++) {
        printf("%u: %s\n", i, list[i]);
    }
}

int main() {
    char words[W][N] = { "a", "BB", "c" };
    convert(W, N, words);
    insert(W, N, words, 0, "start");
    insert(W, N, words, 2, "mid");
    insert(W, N, words, 4, "end");
    insert(W, N, words, 5, "error")
    print(W, N, words);
    return 0;
}

and the output (note: "c" was shifted out as we initially had 3 elements and added 3 new words with valid positions):

0: start
1: a
2: mid
3: bb
4: end

3 Comments

Why is size_t needed in every for loop?
It's just a convenient unsigned type. If you use int then you allow for negative indices which is a source of errors. I use a local loop variable to minimize their scope.
Using size_t is consistent but then change printf("%d: %s\n", i, list[i]); to printf("%zu: %s\n", i, list[i]);

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.