I am trying to write a bool function that checks if the string input has any duplicate characters

Question

I wrote this function under main I want to check if the string input has any duplicate characters but when I used the debug feature it turns out that the outer loop only loops once while the inner loop loops correctly. I don't know why the outer loop shouldn't iterate as many times as the length of the string ?,, im sorry i ran into another problem the program needs to be case insensitive to i tried integrating tolower in the bool function but it does not work for some reason ,yes its a cs50 task

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>
bool only_letters(string s);
bool duplicts(string b);
int main(int argc, string argv[])
{
    if(argc!=2){
        printf("wrong input\n");
        return 1;
    }
    int length=strlen(argv[1]);
    if(length!=26){
        printf("Key must contain 26 characters.\n");
        return 1;
    }
    if(!only_letters(argv[1])){
        printf("key must contain only alphapetic letters.\n");
        return 1;
    }
    if(!duplicts(argv[1])){
        printf("key must not contain same characters\n");
        return 1;
    }
}
// need a check input function if letters or not
bool only_letters(string s){
    int length=strlen(s);
    for(int i =0;i<length;i++){
if(!isalpha(s[i]))
     return false;
    }return true;
}
bool duplicts(string b) {
    int length = strlen(b);
    for (int i = 0; i < length; i++) {
        for (int k = i + 1; k < length; k++) {
            tolower(b[i]);
            tolower(b[k]);
            if (b[i] == b[k])
                return false;
        }
    }
    return true;
}

What is `string`? Is this part of the CS50 course? How do you call this function? Please try to create a [mre] and [edit] your question to show it. — Some programmer dude, May 26 '22 at 11:14
The last thing in the body of your outer loop is `return count`, which leaves the function immediately. Please make a habit of formatting blocks in curly braces `{}` properly. — M Oehm, May 26 '22 at 11:14
On an unrelated note, if you have `if (condition) return true: else return false;` then that's exactly the same as `return condition;`. Since you do the opposite return, then you could do `return !condition;` (which for you translates to `return count <= 0;`) — Some programmer dude, May 26 '22 at 11:18
For starters you should indent your code properly. Even seasoned programmers have a hard time to understand poorly formatted code — Jabberwocky, May 26 '22 at 11:45
[`tolower`](https://en.cppreference.com/w/c/string/byte/tolower) *returns* the new character. — Some programmer dude, May 26 '22 at 12:28
When you edited the code in the question, did you "fix" the problem with the early `return` pointed out in comments and the answer? Then please don't do that, as then your question becomes kind of worthless, since the code doesn't have the problem you ask about, and it also makes our comments and answers equally worthless. — Some programmer dude, May 26 '22 at 12:30
im sorry should i edit it back to what it was before ?, im still new to this so i dont know what is correct from wrong — Mahmoud Herbawi, May 26 '22 at 12:31
Please take some time to read [the help pages](http://stackoverflow.com/help), take the SO [tour], read [ask], as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). — Some programmer dude, May 26 '22 at 12:37

chqrlie · Accepted Answer · 2022-05-26T11:31:45.760

Reformating your code with a sensible indentation convention makes the mistake obvious:

bool duplicts(string b) {
    int length = strlen(b);
    int count = 0;
    for (int i = 0; i < length; i++) {
        for (int k = 1; k < length; k++) {
            if ((b[i]) == b[k])
                count++;
        }
        return count;
    }
    if (count > 0) {
        return false;
    }
    return true;
}

The return count; is inside the outer loop, hence it only iterates once.

There is another problem: the inner loop should start at k = i + 1 otherwise you will always find duplicates in strings of more than 1 byte.

Your code can be easily modified to return false as soon as a duplicate character has been found:

bool duplicts(string b) {
    int length = strlen(b);
    for (int i = 0; i < length; i++) {
        for (int k = i + 1; k < length; k++) {
            if (b[i] == b[k])
                return false;
        }
    }
    return true;
}

You can further simplified to avoid calling strlen and handle arbitrary large strings (albeit very inefficiently):

bool duplicts(const char *s) {
    for (; *s != '\0'; s++) {
        for (size_t k = 1; s[k] != '\0'; k++) {
            if (s[k] == *s)
                return false;
        }
    }
    return true;
}

To fix the quadratic complexity which is inefficient for large strings, you can use an array:

bool duplicts(const char *s) {
    unsigned char seen[256] = { 0 };
    unsigned char c;
    while ((c = *s++) != '\0') {
        if (seen[c] != 0)
            return false;
        seen[c] = 1;
    }
    return true;
}

thank you so much , my string is inputted through the command line and i made an if to exit the program if its more than 26 characters , im sorry but i dont understand the last function , it reduces the time the program might need ?? im still new to programming can you explain it to a potato like me how this new function if more efficient — Mahmoud Herbawi, May 26 '22 at 11:53
The nested for-loops have a time-complexity of O(n²), quadratic. The non-nested while-loop has a time-complexity of O(n), linear. Learn more about the [Big O notation](https://en.wikipedia.org/wiki/Big_O_notation). — Oskar Grosser, May 26 '22 at 12:05

I am trying to write a bool function that checks if the string input has any duplicate characters

1 Answers1