Checking for repeated characters not giving me the desired output

Question

I am trying to use the below code I have written to firstly check if any character in the alphabet has been repeated in string argv[1] regardless if it is upper or lower case. It keeps spitting out a result that I have a duplicate even when I feed it with a command line argument that contains no duplicated characters.

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




int main (int argc, string argv[])
{
    //checking to make sure only one command line argument can be inputted
    if (argc != 2)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

    //defining key argument and making sure string is a certain length
    int key = strlen(argv[1]);

    if (key != 26)
    {
        printf("key is not long enough\n");
        return 1;
    }

    //checking for non alphabetical characters in the key
    for(int i = 0; i < key; i++)
    {
        char n = argv[1][i];
        if (isalpha(n) == 0)
        {
            printf("Your key contains non alpha characters, try again!\n");
            return 1;
        }

        //counting the number of letters in key and storing them in i.
    for(int Argv_letters = 0, key_len = strlen(argv[1]); Argv_letters < key_len; Argv_letters++)
    {
      // making k +1 greater than key letters and adding to k if its less.
      for(int k = Argv_letters + 1; k < key_len; k++)
            if (argv[1][Argv_letters] == argv[1][k] || isupper(argv[1][Argv_letters]) == isupper(argv[1][k]))
            {
                printf("we have duplicate");
                return 1;
            }
            
    }

    return true;

    }

score 1 · Accepted Answer · answered Jan 26 '21 at 15:02

1

You're using isupper(argv[1][Argv_letters]) == isupper(argv[1][k]) which checks that two letters have the same case.

You need to use toupper(argv[1][Argv_letters]) == toupper(argv[1][k]) to check that the two chars represent the same letter.

answered Jan 26 '21 at 15:02

SpectralWave

971
9
18

Note that the parameter of `toupper` is `int`, not `char`. According to [§7.4 ¶1 of the ISO C11 standard](http://port70.net/~nsz/c/c11/n1570.html#7.4p1), the argument must be representable as an `unsigned char` (unless it has the value `EOF`). This means that `toupper(argv[1][Argv_letters])` can lead to undefined behavior on platforms on which `char` is signed (which is the case on most platforms). Therefore, you must cast the `char` to `unsigned char` beforehand. See [this link](https://stackoverflow.com/questions/17975913/does-ctype-h-still-require-unsigned-char) for further information. – Andreas Wenzel Jan 26 '21 at 15:29
@Andrewoca: I believe it was your intention to thank not me, but the person who wrote the answer? All I did was write a comment, pointing out a problem in the answer (which may cause the answer not to work on all platforms). – Andreas Wenzel Jan 26 '21 at 17:38
Thanks @SpectralWave, appreciate it! – Andrewoca Jan 27 '21 at 09:00

Checking for repeated characters not giving me the desired output

1 Answers1