Forcs50
Forcs50

Reputation: 1

Caesar cipher for loop is looping more than expected

first look at my code

#include <stdio.h>
#include <cs50.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>
bool only_digits(string s);

int main (int argc, string argv[])
{
    if (argc !=2) //if more than two CLA inputs, end program.
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    if(only_digits(argv[1]) == false) //if func only_digits returns false, exit prog. 
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    if(only_digits(argv[1]) == true) // if func only_digits returns true, print argv[1].
    {
        printf("Text: %s\n", argv[1]);
    }

}
bool only_digits(string s)    //func taking string input and bool output
{
    for(int i = 0; i < strlen(s); i++)  // loop till strlen(s)
    {
        printf("strlen(s) = %lu\n",strlen(s)); //debug
        printf("s[%d] = %c is alpha\n",i, s[i]);  //debug

        if(isalpha(s[i]) == !0)  //inside loop check each char of string s in isalpha.
        {
            printf("s[i] = %c is alpha\n",s[i]); //debug
            return false;                         //if char is alpha, return false to function
        }

    }
    return true;    // if not returned false inside loop, return true.

}

My for loop runs from i=1 to i=strlen() then loops again to from i=0 to i=strlen() that is it loops 2*strlen() times instead of strlen() times. Here is the CLA and output

caesar/ $ ./caesar b2
strlen(s) = 2
s[0] = b is alpha
strlen(s) = 2
s[1] = 2 is alpha
strlen(s) = 2
s[0] = b is alpha
strlen(s) = 2
s[1] = 2 is alpha
Text: b2

Point Wise Questions:

  1. Why is my if condition inside for loop not working as expected (return false when encountering an alphabet char.)?
  2. Why is my for loop iterating twice times more than expected?
  3. What is wrong with this code?

Upvotes: 0

Views: 104

Answers (1)

Vlad from Moscow
Vlad from Moscow

Reputation: 311038

You are calling the function only_digits in main twice due to conditions in these if statements

    if(only_digits(argv[1]) == false) //if func only_digits returns false, exit prog. 
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    if(only_digits(argv[1]) == true) // if func only_digits returns true, print argv[1].
    {
        printf("Text: %s\n", argv[1]);
    }

This if statement

if(isalpha(s[i]) == !0)

is equivalent to the statement

if(isalpha(s[i]) == 1)

However it is not necessary that the function isalpha will return exactly 1 in case when an alpha character is encountered. It just returns a non-zero value. You need at least to write

if ( isalpha( ( unsigned char )s[i] ) != 0 )

or just

if ( isalpha( ( unsigned char )s[i] ) )

Pay attention to that the string can also contain other symbols excepts alpha and digit symbols.

Also there is no great sense to call the function strlen several times.

Using your parameter declaration the function only_digits can be defined the following way

bool only_digits( string s ) 
{
    while ( *s && isdigit( ( unsigned char ) *s ) ) ++s;

    return *s == '\0';
}

Upvotes: 1

Related Questions