function returning values it isn't supposed to

so i have a program that first has a aplhabetType_ value.this can be 26,30 or 40.
26 is basic alphabet,30 has 4 special characters,and 40 has numbers aswell.
this is the function that transaltes a number to a character.
i'm pretty sure everything is handled here.
but it obviously isn't cause im getting errors
if alphabet type is 26,then it should return only numbers from 0-25,and all other values passed into the function should be returned as '23'<the letter x
if alphabet type is 30,then return numbers from 0-29,and so on.
i cant see an error in the logic,but there must be!
please help me find it
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
int Simpleciphers::toNum(char x){
x=tolower(x);

int i=(int)x;
switch (alphabetType_){
    case 40:

if(i<97||i>122){
 if(i>=48&&i<=57){

return x-18;


}
switch(i){
case 46:
    return 26;
    break;
case 44:
    return 27;
    break;
case 63:
    return 28;
    break;
case 33:
    return 29;
    break;
default:
    return 23;

}


}else if(i>=97&&i<=122) {
return x-97;
}else{
return 23;

}
break;
case 30:

if(i<97||i>122){
switch(i){
case 46:
    return 26;
    break;
case 44:
    return 27;
    break;
case 63:
    return 28;
    break;
case 33:
    return 29;
    break;
default:
    return 23;

}


}else if(i>=97&&i<=122) {
return x-97;
}else{
return 23;

}
break;
case 26:

if(i>=97&&i<=122) {
return x-97;

}else{
return 23;

}
break;
}
}
I've just done a little formatting to make this easier for me to follow what's happening. What errors are you getting?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
int Simpleciphers::toNum(char x)
{
    x=tolower(x);
    int i=(int)x;
    
    switch (alphabetType_)
    {
        case 40:
                if(i<97||i>122)
                {
                    if(i>=48&&i<=57)
                    {
                        return x-18;
                    }
                    switch(i)
                    {
                        case 46:
                                return 26;
                                break;
                        case 44:
                                return 27;
                                break;
                        case 63:
                                return 28;
                                break;
                        case 33:
                                return 29;
                                break;
                        default:
                                return 23;
                    }
                }
                else if(i>=97&&i<=122) 
                {
                    return x-97;
                }
                else
                {
                    return 23;
                }
                break;
        case 30:
    
                if(i<97||i>122)
                {
                    switch(i)
                    {
                        case 46:
                                return 26;
                                break;
                        case 44:
                                return 27;
                                break;
                        case 63:
                                return 28;
                                break;
                        case 33:
                                return 29;
                                break;
                        default:
                                return 23;
                    }
                }
                else if(i>=97&&i<=122) 
                {
                    return x-97;
                }
                else
                {
                    return 23;
                }
                break;
        case 26:
                if(i>=97&&i<=122) 
                {
                    return x-97;
                }
                else
                {
                    return 23;
                }
                break;
    }
}
Could condense this code a lot, if you do one basic comparison for i between 'a' and 'z' first, then address the other two cases with a few lines, and finally fall through to returning 'x' - 'a' as the last resort.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
{
	x= tolower(x);
	if(x >='a' &&  x<='z') // lower alpha characters; included in all alphabets;return 
		return x - 'a';
	if(alphabetType_== 40) // include numbers 
	{
		if(x >='0' && x <= '9')
			return x - 18;
	}
	if((alphabetType_== 40)||(alphabetType_== 30))
	{
		switch(x) //four special cases 
		{ 
		case 46:
			return 26;
			break;
		case 44:
			//etc.. . . 
		}
	}
	return 'x' - 'a';
}


Still not optimal, but doable, and gives you a shot at inserting some diagnostic cout << at critical points. Fewer points to troubleshoot, too.

I'm not a big fan of multiple returns, either but that's going to be a never-ending discussion, and in the long run it's programmer preference, in my opinion.

Last edited on
turns out i am not getting any errors,it was something else in the program
thanks for the formatting though
it definetly looks better
its something i really need to work on,making ugly code is really bad.
as for crumley,thank you for showing me how to do that.
i had no idea you could compare characters like that.
also i have a toChar function that converts 23>x,by adding 97 to it
so x is 23 starting from 0 ofcourse,and in my program.
Last edited on
You're using a whole lot of magic constants. Here is the code without them. Notice that it's much easier to see what's going on now:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
int Simpleciphers::toNum(char x)
{
    x=tolower(x);
    int i=(int)x;

    switch (alphabetType_)
    {
        case 40:
                if(i<'a'||i>'z')
                {
                    if(i>='0'&&i<='9')
                    {
                        return x-18;
                    }
                    switch(i)
                    {
                        case '.':
                                return 26;
                                break;
                        case ',':
                                return 27;
                                break;
                        case '?':
                                return 28;
                                break;
                        case '!':
                                return 29;
                                break;
                        default:
                                return 23;
                    }
                }
                else if(i>='a'&&i<='z')
                {
                    return x-'a';
                }
                else
                {
                    return 23;
                }
                break;
        case 30:

                if(i<'a'||i>'z')
                {
                    switch(i)
                    {
                        case '.':
                                return 26;
                                break;
                        case ',':
                                return 27;
                                break;
                        case '?':
                                return 28;
                                break;
                        case '!':
                                return 29;
                                break;
                        default:
                                return 23;
                    }
                }
                else if(i>='a'&&i<='z')
                {
                    return x-'a';
                }
                else
                {
                    return 23;
                }
                break;
        case 26:
                if(i>='a'&&i<='z')
                {
                    return x-'a';
                }
                else
                {
                    return 23;
                }
                break;
    }
}


The problems are clearer now too. The else at line 39 is never executed: a character is either outside the a-z range (line 9) or inside (line 33).

As PCrumley48 said, this can be simplified a lot. I'd map the character to the 40-letter alphabet and then modify it if it's part of the smaller ones:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
// map character x to 26, 30, or 40 character alphabet.
// 26 character alphabet maps 'a'-'z' to 0-25.
// 30 character alphabet adds '.', ',', '?' and '!' to
// mapped to 26-29
// 40 character alphabet adds '0' - '9' mapped to 30-39
int toNum(char x)
{
    x=tolower(x);
    int result = 'x' - 'a';// default is 'x'

    // map to 40 character alphabet
    if (x >= 'a' && x <= 'z') {
        result = x - 'a';
    } else if (x >= '0' && x <= '9') {
        result = x - '0' + 30;
    } else {
        const char *cp = ".,?!";
        char *pos = strchr(cp, x);
        if (pos) {
            result = pos-cp + 26;
        }
    }

    if (result >= alphabetType_) {
        result = 'x' - 'a';
    }
    return result;
}

i dont understand lines 16-21
could you explain further what they do?also what are magic numbers?
i read on wikipedia but i can't quite grasp the concept

and i can't use anything except standard library,in which strchr doesnt seem to be.
Last edited on
17: creates a const string of the 4 special characters.
18: scans the string of 4 special characters checking if x is one of the 4 special characters. http://www.cplusplus.com/reference/cstring/strchr/
If x exists in the string, strchar returns a pointer to the position in the string or null if it wasn't found.
19: Tests to see if x was found.
20 If x was found, (pos-cp) calculates the offset (0-3) of x within the string of special characters by subtracting the base address of the string from the pointer to the char found, then 26 is added to give values of 26-29 which is stored in result.

stdchr is part of the C library.
Last edited on
stdchr is part of the C library.
true, but it lives in <cstring>, so to use it you have to #include <cstring>

http://www.cplusplus.com/reference/cstring/
Topic archived. No new replies allowed.