5

The following line has the problem int (*f)(int, int) = (argv[2][0] == 'd') , on compiling it says declaration not allowed here . Should the line be declared at the start , any better way of doing this .Any suggestions would be greatly appreciated?

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

int encode(int ch, int key) { 
        if (islower(ch)) {
                ch = (ch-'a' + key) % 26 + 'a';
                ch += (ch < 'a') ? 26 : 0;
        }
        else if (isupper(ch)) {
                ch = (ch-'A' + key) % 26 + 'A';
                ch += (ch < 'A') ? 26 : 0;
        }
        return ch;
}

int decode(int ch, int key) { 
        return encode(ch, -key);
}

int main(int argc, char **argv) { 
        int ch;
        int key;

        if (argc < 2) {
                printf("USAGE: cipher <integer key> <encode | decode>\n");
                printf("Then, just type your text and it will automatically output the en/de crypted text! :)\n");
                return 1;
        }

        key = atoi(argv[1]);
        if (key < 1 || key > 25) {
                printf("Key is invalid, or out of range. Valid keys are integers 1 through 25.\n");
                return 1;
        }

        int (*f)(int, int) = (argv[2][0] == 'd') ? 
                decode : 
                encode;

        while (EOF != (ch=getchar()))
                putchar(f(ch, key));

        return 0;
}
Nik Bougalis
  • 10,495
  • 1
  • 21
  • 37
Ashwin Singh
  • 67
  • 1
  • 3
  • 10

4 Answers4

15

In C (prior to C99), you have to declare variables at the start of a block.

Either compile your code as C99, or change the code so that f is declared at the start of a block.

NPE
  • 486,780
  • 108
  • 951
  • 1,012
2

In c89/90 You must declare all the variables in the starting of the block

But In c99 , You can compile your code with -std=c99 like this:

gcc -Wall -std=c99 test.c -o test.out

Omkant
  • 9,018
  • 8
  • 39
  • 59
  • Your code is c++ tagged so I think you should try it in g++ , And in C++ it's allowed – Omkant Nov 28 '12 at 17:39
  • @Aniket : You are seeing this late.. It was previously C++ also – Omkant Nov 28 '12 at 17:43
  • 1
    you would never get this error if it were C++. Since the OP mentioned /not allowed declaration/ its obviously C(and assumed by almost every other answerer) – Aniket Inge Nov 28 '12 at 17:44
  • @Aniket : That's why I answered it and mentioned that c++ things in comments , But now it's all edited so there is nothing to discuss – Omkant Nov 28 '12 at 17:47
  • @Aniket : See the answers agreed or not ? , comments are just a kind of chat – Omkant Nov 28 '12 at 17:48
2

Other than the part pointed out by NPE, you can use typedef to create a Function Type. like this:

typedef void FunctionType (int, int); And then use it(as a separate type) to create function pointers.

Makes reading easy.

Aniket Inge
  • 25,375
  • 5
  • 50
  • 78
  • 1
    Downvoted (because it doesn't answer the main question) but then changed my mind (the question does say "*Any* suggestions"). I think code review questions are now off-topic, so if the question were properly focussed then this would be a comment not an answer. – Steve Jessop Nov 28 '12 at 17:40
  • @SteveJessop true. I wouldn't have answered this question seeing NPE's answer. But since the OP mentioned "Suggestions" I thought I would throw in my 2 cents – Aniket Inge Nov 28 '12 at 17:41
  • agreed, that's why I changed my mind. If the question shouldn't say "any suggestions" then that's between the questioner and the electorate, not you and the electorate ;-) – Steve Jessop Nov 28 '12 at 17:44
  • Never understood people that cannot read C declaration syntax... I find it very clear and intuitive. – Yakov Galka Nov 28 '12 at 18:05
  • @ybungalobill really? try this: `T (*(*pfe)())[]` :-P – Aniket Inge Nov 28 '12 at 18:09
  • @Aniket: a pointer to a function with no arguments returning a pointer to a pointer (`[]`) to T. (This was from my head.) This is straight forward once you understand that the declaration syntax closely follows the expression syntax. – Yakov Galka Nov 28 '12 at 18:19
  • @ybungalobill and that expression was straight from my head as well, just typed in as I thought about it. My point is, it might be easy for you and me, but not to Bob and Joe(our testers). They might either ask us to refactor it or simplify it. – Aniket Inge Nov 28 '12 at 18:25
2

Should the line be declared at the start

In C89 definitions must occur before any statements in the block. If you do move it, you don't have to move the whole line (and of course you don't want to move the whole line to before the code that checks argv[2] is valid). Just move the definition of f:

    int ch;
    int key;
    int (*f)(int,int);

    ...

    f = (argv[2][0] == 'd') ? decode : encode;

any better way of doing this

It's not necessarily better in this case, but note that the rule is the start of a block, not necessarily the start of a function.

So, you could just write:

{
    int (*f)(int, int) = (argv[2][0] == 'd') ? 
            decode : 
            encode;

    while (EOF != (ch=getchar()))
            putchar(f(ch, key));
}
return 0;

You can easily get into arguments about this coding style. Some people think every function should define all its variables up front, and that introducing a block just to define a variable is cluttered and/or confusing. Some people (and especially those who use C++ as well as C) think you should restrict the scope of each variable to as narrow a piece of code as possible, that that defining everything at the start of the function is cluttered and/or confusing. But even they might consider a bare block excessive.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699