C++

Forum

 
frustrated cause i write bad code..

DAoliHVAR (77)
As you can see by me posting in the beginner section,i am a beginner.
but for all the code that i write i have a feeling(and most often right)that the code that i have written is 1.ugly 2.can be written in a much shorter better manner.
now the 1st one i suppose will come with experience.
the second problem though,is with the way i implement things.Is this a problem with me not being smart enough to find optimal solutions for problems,or will it also come with experience?
is there a way i can teach myself to think so that i can find optimal solutions for things?
consider the following code,showing my implementation of the rail fence cipher.if you can't be bothered to read it I don't blame you.
it basically counts from zero>number then from number back to zero,cause that's sort of how rail fence works 
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
 
while(true){

    for(int i = 0;i<key;i++){
            char tc = rawText[encryptedLetterCounter];
        if(isspace(tc)){
            if(haveSpaces_==true){
                  encryptionMatrix[i][columnCounter]=tc;
                  encryptedLetterCounter++;

                      if(encryptedLetterCounter==t){
                        breakFlag=true;
                        break;
                        }

                     }else{
            i--;

            encryptedLetterCounter++;

                 if(encryptedLetterCounter==t){
                breakFlag=true;
                break;
                }

            }
            continue;
        }
        encryptionMatrix[i][columnCounter]=tc;
        encryptedLetterCounter++;
       

        if(encryptedLetterCounter==t){
            breakFlag=true;
            break;
        }
    }
    if(breakFlag==true){

        break;

    }
    columnCounter++;
     for(int i = (key-2);i>=1;i--){

          char tc = rawText[encryptedLetterCounter];
        if(isspace(tc)){
            if(haveSpaces_==true){
                  encryptionMatrix[i][columnCounter]=tc;
                  encryptedLetterCounter++;
               
                      if(encryptedLetterCounter==t){
                        breakFlag=true;
                        break;
                        }

            }else{
            i++;
            encryptedLetterCounter++;
                if(encryptedLetterCounter==t){
                breakFlag=true;
                break;
                }

            }
            continue;

        }
        encryptionMatrix[i][columnCounter]=tc;
        encryptedLetterCounter++;

        if(encryptedLetterCounter==t){
          breakFlag=true;
            break;
        }

    }

        if(breakFlag==true){
            break;
        }

        columnCounter++;
}

now it gets the job done,but its long and ugly.
can somebody use this to show me what i could have done better
and what i should look to improve in the future?
Last edited on
TheIdeasMan (6817)
Hi,

Functions:

There is a kind of rule that ones functions shouldn't be more than 40 Lines of Code (LOC)

Candidates for functions include the bodies of loops, and functions should only do one thing. They also help readability and understanding.

Also, if you are writing the same or very similar code over and over, then that code should be organised into functions.

With those things in mind, can you see how to improve your code?

Hope all is well :+)
DAoliHVAR (77)
yeah i see.but i can't find a way to actually part my loops into functions.
unless its like small one line things
like this

1
2
3
4
 
   if(encryptedLetterCounter==t){
                        breakFlag=true;
                        break;
                        }


but would that be better?
declaring a whole function to save a couple of lines in the loop
fabtasticwill (218)
When you do if(breakFlag == true), instead just do if(breakFlag). The == true is unnecessary. Same thing with false. You can do if(!condition) instead of if(condition == false)
Topic archived. No new replies allowed.