3
\$\begingroup\$

Currently, I have a function that loops to gather input for program execution, but I feel that it is a bit lacking, overthought and unoptimized. I would love any input on the process, coding style, or usability of the function.

I feel like the use of while (true) is not really optimized here.

My code uses curses and unistd.h.

// Heart of the display of the program
void startEvolutionLoop (Grid* grid, size_t speed) {
    clear();

    // Infinitely loops until 'q' is pressed
    while (true) {
        // First of all displays the grid with supporting information
        displayGrid(grid);
        printw("[Space] Evolve once        [Enter] Run evolutions        [q] Exit\n");

        // Gets the most recently touched character
        int c = getch();

        //  - if space (' ') evolves and loops again
        //  - if ('q') ends the loops
        //  - if enter ('\n') starts a sub-loop similar to the main one, but waits for any character to stop
        if (c == ' ') {
            grid->evolve();
        } else if (c == 'q') {
            return;
        } else if (c == '\n') {
            nodelay(stdscr, TRUE); // Getting of the input is asyncronous
            // Infinitely loops until any key is pressed
            while (true) {
                grid->evolve();
                displayGrid(grid);

                printw("Press any key to end execution...                       \n"); // Extra whitespace to override previously written stuff in this area

                // If there has been something inputted, then break out of this sub-loop
                if (getch() != ERR) {
                    break;
                }

                usleep((__useconds_t)speed * 1000); // Sleep the desired amount of time (in micro seconds converted to milliseconds)
            }
        }
    }
}

For context (which is not necessary), you can check out my GitHub Repo.

\$\endgroup\$
1
  • \$\begingroup\$ What exactly do you want to optimize in the loop, speed or memory usage? \$\endgroup\$ – pacmaninbw Apr 9 '17 at 13:43
5
\$\begingroup\$

Consider following the Single Responsibility Principal and break the function up into smaller functions that do less.

Note: This answer makes use of the loopEvos() that is provided in the updated version of the source files on GitHub.

void loopEvos (Grid* grid, size_t speed) {
    nodelay(stdscr, TRUE); // Getting of the input is asyncronous
    // Infinitely loops until any key is pressed
    while (true) {
        grid->evolve();
        displayGrid(grid);

        printw("Press any key to end execution...\n");

        // If there has been something inputted, then break out of this sub-loop
        if (getch() != ERR) {
            break;
        }

        usleep((__useconds_t)speed * 1000); // Sleep the desired amount of time (in micro seconds converted to milliseconds)
    }
}

Whenever code is written future expansion of the program/function must be considered. In the function above there is the code :

        if (c == ' ') {
            grid->evolve();
        } else if (c == 'q') {
            return;
        } else if (c == '\n') {
            loopEvos(grid, speed);
            nodelay(stdscr, FALSE);
        }

It would be is easier to expand the code if another type of control statement was used instead. If the code was

        switch (c) {
            case ' ':
                grid->evolve();
                continue;
            case 'q':
                return;
            case '\n':
                loopEvos(grid, speed);
                nodelay(stdscr, FALSE);
                continue:
            default :
                printw("Invalid character input\n");
                return;
        }

It would be easier to handle any additional input characters such as KEY_UP, KEY_LEFT, KEY_DOWN and KEY_UP.

        switch (c) {
            case ' ':
                grid->evolve();
                continue;
            case 'q':
                return;
            case '\n':
                loopEvos(grid, speed);
                nodelay(stdscr, FALSE);
                continue;
        // The following code is modified from procs/procs.hpp in the GitHub account.
            case 's':
                askFileSave(grid);
                continue;
            case 'c':
                changeSettings(&speed);
                continue;
            case KEY_UP:
                if (cursorPosY >= 1) {
                    cursorPosY -= 1;
                }
                continue;
            case KEY_LEFT:
                if (cursorPosX >= 2) {
                    cursorPosX -= 2;
                }
                continue;
            case KEY_DOWN:
                if (cursorPosY < (grid->y - 1)) {
                    cursorPosY += 1;
                }
                continue;
            case KEY_RIGHT:
                if ((cursorPosX / 2) < grid->x - 1) {
                    cursorPosX += 2;
                }
                continue;
            default :
                printw("Invalid character input\n");
                return;
        }

It might be easier to read, understand and modify the outer loop if it was a do while loop rather than a while true loop

void startEvolutionLoop (Grid* grid, size_t speed) {
    clear();

     int userInput;
     do {
         // First of all displays the grid with supporting information
         displayGrid(grid);
         printw("[Space] Evolve once        [Enter] Run evolutions        [q] Exit\n");

         // Gets the most recently touched character
         userInput = getch();

         if (userInput == ' ') {
             grid->evolve();
         } else if (userInput == 'q') {
             continue;
         } else if (userInput == '\n') {
             loopEvos(grid, speed);  // from procs/procs.hpp in the GitHub account.
             nodelay(stdscr, FALSE);
         }
     } while (userInput != `q`);
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you for the response! The project was mostly a learning demo, but I'll be sure to take your notes into consideration for the future. \$\endgroup\$ – Ziyad Edher Apr 12 '17 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.