HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Draw random 1's and 0's

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomdrawand

Problem

I was wondering if anyone would be so kind as to look at this code I've written for practice. Tell me what am I doing wrong, where can I optimize, anything.

What this code does is (in terminal) draw a bunch of random 1's and 0's, but I added an effect kinda like a lattice. The bulk of this I got help on here.

```
#include
#include
#include

int main (int argc, char *argv[]){ //check for whitch effect to print
switch(*argv[1]){
case '1': lattus();break;
case '2': normal();break;
default: printf("1 = lattus effect | 2 = static effect\n");break;
}
}

char randstring (char buffer, int length){ //genertate a random number
int i = length;
while(--i >= 0) {
buffer[i] = (rand() % 2) ? '1' : '0';
}
buffer[length] = 0;
return buffer;
}

int normal(){ // normal drawing of 1's and 0's
struct winsize w;
ioctl(0, TIOCGWINSZ, &w);
int width = w.ws_col;
int height = w.ws_row; //get terminal width and height
char buffer[width*height + 1]; //create a buffer big enough to hold one draw to the screen
int i = 25;
while(i-- >= 0) {
printf("%s\n", randstring(buffer, width*height)); //draw to screen
usleep(90000);
}
system("clear"); //clear screen
}

int lattus (void){
struct winsize w;
ioctl(0, TIOCGWINSZ, &w);
int width = w.ws_col; //get the terminal width
char buffer1[width + 1]; //create 3 buffers for each segment
char buffer2[width + 1]; //each big enough to hold the width of the terminal
char buffer3[width + 1];
int first = 1; //how many before the space
int second = width - 8; //how many in the middle of the space
int third = 1; //how many at the end of the space
int i = 1000; //draw 1000 lines
int on = 0; //switch for growing and shrinking
while(i-- >= 0){
usleep(9000);
if(first == 1 && third == 1 && second == wi

Solution

A few pointers:

First, variable length arrays are only allowed in C99:

char buffer1[width + 1];


If the code is to be strictly ANSI C, one would make an pointer to an array and use a function like malloc or calloc to allocate some memory for the array:

char* buffer = malloc(sizeof(char) * (width + 1));


(Then, if necessary, the buffer would be initialized, or calloc used instead to initialize and allocate at once.)

Second, code readability:

if(second % 2 == 0){
    second = second - 2;
}else{
    second = second - 3;
}


Could be better written as:

if (second % 2 == 0) {
    second = second - 2;
} else {
    second = second - 3;
}


It may seem like nitpicking, but it does make it easier for people to read your code and understand what is going on. Coding is a form of communication as well -- make it easier for others to read, and it others will be able to better understand what you're trying to achieve.

The switch statement could be similarly improved:

switch(*argv[1]){
    case '1': lattice();break;
    case '2': normal();break;
    default: printf("1 = lattice effect | 2 = static effect\n");break;
}


To:

switch(*argv[1]) {
    case '1':
        lattice();
        break;
    case '2':
        normal();
        break;
    default:
        printf("1 = lattice effect | 2 = static effect\n");
}


Also, adding line breaks between certain blocks of statement which perform one task may improve readability.

There seems to be copious use of while loops, but I don't see a single for loop -- for loops are probably more useful for performing "counter"-type activities, such as performing repetitions for a certain number of iterations:

int i;
for (i = 0; i < 100; i++) {
    /* Action to perform 100 times */
}


The basic structure of a for loop is as follows:

for (initial-condition; loop-condition; expression-on-each-iteration) {
    /* Action to perform on each iteration. */
}


(Note: The name for the conditions are not official names, I just made them up to describe what they're for.)

The initial-condition is what to do when the for loop first begins. Many times, this is used to initialize a counter variable to some value. For example, i is set to 0.

The loop-condition is an expression that is evaluated at the beginning of each iteration which determines whether the loop should continue. If the expression is true, then the loop is performed, but if false, then the loop terminates.

The expression-on-each-iteration is something that is performed at the beginning of each iteration of the loop, if that iteration is to take place. Commonly, this is part is used to increment the counter used to control the loop.

Code Snippets

char buffer1[width + 1];
char* buffer = malloc(sizeof(char) * (width + 1));
if(second % 2 == 0){
    second = second - 2;
}else{
    second = second - 3;
}
if (second % 2 == 0) {
    second = second - 2;
} else {
    second = second - 3;
}
switch(*argv[1]){
    case '1': lattice();break;
    case '2': normal();break;
    default: printf("1 = lattice effect | 2 = static effect\n");break;
}

Context

StackExchange Code Review Q#4275, answer score: 14

Revisions (0)

No revisions yet.