C program doesn't work correctly

Question

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


void main()
{
    char alfavita[30] =
    {
        'a',
        'b',
        'c',
        'd',
        'e',
        'f',
        'g',
        'h',
        'i',
        'j',
        'k',
        'l',
        'm',
        'n',
        'o',
        'p',
        'q',
        'r',
        's',
        't',
        'u',
        'v',
        'w',
        'x',
        'y',
        'z'
    };

    char str[20];


    printf("Give a word:\n");
    gets(str);


    for(int i=0;i<strlen(str);i++)
    {
        for(int j=0;j<strlen(alfavita);j++)
            if(alfavita[j] == str[i])
                str[i] = alfavita[j+3];
    }



    puts(str);
}

For example if i give 'a' it should be return 'd' (each letter will transform into the 3d next of the alfavita array ) But it just prints me a null string. I can't find something wrong or I don't see it .

What did you find out so far, such that we don't waste precious time on things you have already verified? — Sebastian Mach, Jun 03 '14 at 11:58
`str[i]` is the ASCII value for the character at that point. When fed with a lowercase character such as `a`, it's looking for a 97th entry. — Jongware, Jun 03 '14 at 11:58
You can have out of bound error on inputs like 'zzz'; something like `(j+3) % 30` should be used — Dmitry Bychenko, Jun 03 '14 at 12:01
what if `if(alfavita[j] == str[i])` become true when `j` index point last character. — Jayesh Bhoi, Jun 03 '14 at 12:02
You can define alfavita as `char alfavita[30] = "abcdefghijklmnopqrstuvwxyzabc"` — Mohit Jain, Jun 03 '14 at 13:31

Nicolas Defranoux · Answer 1 · 2014-06-03T12:04:48.240

1

str[i] = alfavita[j+3];
After this line the code continues, so it will put i+3, i+6, ... until it gets out of alfavita.
You can add a break to exit the inner loop like that:

for(int i=0;i<strlen(str);i++)    
{
    for(int j=0;j<strlen(alfavita);j++)
        if(alfavita[j] == str[i])
        {
            str[i] = alfavita[j+3];
            break;  // next i.
        }
}

, or maybe just directly access the array:

for(int i=0;i<strlen(str);i++)
{
  char c = str[i];
  if (c >= 'a' && c <= 'z') {
    str[i] = alfavita[(c - 'a' + 3) % strlen(alfavita)];
  }
}

Note the % strlen(alfavita) to avoid ending after the end of the list. You could also write it:

if (c >= 'a' && c <= 'z') {
  str[i] = ((c - 'a' + 3) % 26) + 'a';
}

edited Jun 03 '14 at 12:04

answered Jun 03 '14 at 11:59

Nicolas Defranoux

2,646
1
10
13

1

It's pretty bad form to recommend code that locks the program to a particular encoding. You cannot assume that the letters are continuously laid out. – unwind Jun 03 '14 at 12:27
Strictly reading the standard that's true, though I don't know any C encoding where the letters a to z are not contiguous. – Nicolas Defranoux Jun 03 '14 at 14:14
2

[EBCDIC](http://en.wikipedia.org/wiki/EBCDIC) is the standard example. Of course it's not widely used, but it does exist. – unwind Jun 03 '14 at 14:16
So you can make a slow code that will run on every machine, or a faster code that works only on 99.99 % of the machines... – Nicolas Defranoux Jun 03 '14 at 14:25

unwind · Answer 2 · 2014-06-03T14:29:07.063

You can use a table that gives the replacement character for each character.

Then encode by computing the index into plain, and transferring that index into encoded:

char encode_char(char c)
{
  const char *plain   = "abcdefghijklmnopqrstuvwxyz";
  const char *encoded = "defghijklmnopqrstuvwxyzabc";

  const char *pp = strchr(plain, c);
  if(pp != NULL)
    return encoded[(ptrdiff_t) (pp - plain)];
  return '?';
}

How the above works:

Define two strings that are supposed to be 1:1 mapped, i.e. plain[0] is encoded into encoded[0]. This can be more clearly modelled (i.e. by a struct that has the pair) but then the iniialization becomes more complicated.
Search for the input character c inside the plain string. This returns NULL if not found, or a pointer to somewhere inside plain found.
Make sure the pointer isn't NULL before using its value.
Subtract plain (i.e. &plain[0], the address of the a) from pp. This evaluates to 0 for a, 1 for b, and so on.
Use the computed index to look up the corresponding character in encoded.
On failure to encode, return ?.

In a portable, general program, you can not use plain subtraction (i.e. c - 'a'), since C does not guarantee that characters are encoded in values next to each other.

As pointed out, the above assumes that each character encodes in exactly one char. That might not be true for targets with exotic encodings, in which case it really is safer to use an explicit table, like this:

const struct {
  char plain;
  char encoded;
} encoding[] = {
 { 'a', 'd' },
 { 'b', 'e' },
 { 'c', 'f' },
 /* ... and so on ... */
};

The encoding function then becomes:

char encode_char2(char c)
{
  for(size_t i = 0; i < sizeof encoding / sizeof *encoding; ++i)
  {
    if(encoding[i].plain == c)
      return encoding[i].encoded;
  }
  return '?';  /* Not found. */
}

Can you please explain your answer a little? To me it seems like `pp - plain` should be replaced with `*pp - 'a'` or I am missing something. — Mohit Jain, Jun 03 '14 at 13:29
pp is pointer (char*) to the found character, plain is a pointer to the beginning of the plain string, so pp - plain is the difference between the points, that's an offset within the string. encoded[offset] is valid, you could also write *(encoded + offset). — Nicolas Defranoux, Jun 03 '14 at 14:17
This approach will not work with UTF-8 variable lengh encoding though. — Nicolas Defranoux, Jun 03 '14 at 14:18
@NicolasDefranoux But a..z all encode in a single byte in UTF-8. — unwind, Jun 03 '14 at 14:24
Sure, but if you use UTF-8 it's usually to add extra characters. — Nicolas Defranoux, Jun 03 '14 at 14:28
@Nicolas: so you can make a slow routine that works for every possible encoding, or a fast one that works as asked for `a..z`. — Jongware, Jun 03 '14 at 15:17

C program doesn't work correctly

2 Answers2