0

So I am trying to print a board exactly like this with the multidimensional array

char score[10][10] = {' '};

   a b c d e f g h i j
  +-------------------+
 0|                   |
 1|                   |
 2|                   |
 3|                   |
 4|                   |
 5|                   |
 6|                   |
 7|                   |
 8|                   |
 9|                   |
  +-------------------+

Currently this is my code:

#include <iostream>
using namespace std;
#include <vector>
#include <string.h>

int main() {
    char score[10][10] = {' '};

    cout << "   a b c d e f g h i j" << endl;
    cout << "  +-------------------+" << endl;

    for (int i = 0; i < 10; i++) {
        cout << " " << i << "|";
        for (int j = 0; j < 10; j++) {
            cout << score[i][j];
        }
        if(i == 0) {
            cout << "                  |";
        } else {
            cout << "                   |";
        }
        cout << endl;
    }
    cout << "  +-------------------+" << endl;
}

As you can see my code is inefficient and lengthy.

What would be the most efficient possible way ( or a more efficient way) of printing the board as exactly shown above with the multidimensional score array?

Felix Glas
  • 15,065
  • 7
  • 53
  • 82
  • Might want to try here: http://codegolf.stackexchange.com/ –  May 25 '14 at 21:47
  • Other than the indentation is broken, this code looks fine. – Oliver Charlesworth May 25 '14 at 21:48
  • 7
    I'd just like to point out that short code doesn't always mean efficient code and vice versa – Matt Coubrough May 25 '14 at 21:52
  • 3
    Is this proving to be a bottle neck in your code? If not I would just fix the indentation and use this. It's clear and concise and I doubt it's too slow. – GWW May 25 '14 at 21:54
  • 2
    I don't know why are you talking to "efficiency" in a so simply code??? – Elvis Dukaj May 25 '14 at 21:58
  • 1
    That code doesn't work properly, by the way. At least, it only works if the first and only char in the 2d array is a space. ( If you're not going to use the array, you can get away with a much less intricate inner loop consisting of just cout << " " << i << "| |" << '\n'; (you'll need to add some spaces this site removed from between the two bar characters there) ) –  May 25 '14 at 22:04
  • @Poldie His code will work fine. I think a better way to put it is that his table may have unexpected characters, not just spaces. Saying his code doesn't work implies it will throw an error during compile or run-time. – Daniel May 25 '14 at 22:07
  • 1
    @Daniel Populate his array, with the code as-is, and see what you get. Yes, a run-time error; the wrong result - a badly formatted table. –  May 25 '14 at 22:08
  • For the record: your code is neither inefficient nor lengthy. (Only the `if(i == 0)` part seems .. redundant? An error?) – Jongware May 25 '14 at 22:19
  • @Jongware It's to cope with that single space in the array I keep going on about! The code needs a tweak to handle the array being populated properly. –  May 25 '14 at 22:22
  • @Poldie What do you mean "populate his array"? His code compiled and runs fine for me. Yes, he should be using `{{' '}}` and it will only set the first char to a space but it's still compilable as-is. The uninitialized values should still print out as characters, they will just be pseudo-random. – Daniel May 25 '14 at 22:23
  • this question should be in code review http://codereview.stackexchange.com/ – Bryan Chen May 25 '14 at 22:24
  • @Jongware `if (i == 0)` is not redundant, your thinking of `if (i != 0)`, either way, `i` is an integer and should be treated as thus. Doing things like `if (!i)` only obscures the meaning of `i` and is hardly quicker than a comparison. – Daniel May 25 '14 at 22:25
  • 1
    @Daniel I'm assuming this code is supposed to display the contents of that array when it's populated; for example, to output the current state of a Battleships game. As it stands it won't do that. If you fill it with, for example, a mixture of 'o' and 'x' the output is wrong. He fills the array with binary zero except for the first char, so it generally outputs nothing. Why do you think he has to print all those spaces in each line? –  May 25 '14 at 22:29

3 Answers3

1

As the comments have pointed out, your code is almost efficient as possible. Making it shorter would do little to its run-time and instead obscure its meaning. There are however a few things you can do to speed it up.

  1. Avoid an extra call to operator<<, evaluation of std::end, and unnecessary buffer flushes by using \n and including it in your existing string literals (evaluated at compile time).

  2. Use printf instead of cout. See "performance" section of this article.

Community
  • 1
  • 1
Daniel
  • 1,920
  • 4
  • 17
  • 35
  • In the old days, we would overload endl to handle record oriented streams where '\n' would just be another character. – user3344003 May 25 '14 at 22:52
0

As already pointed out by others, there are not many ways to make this more efficient, particularly if constrained to use the cout stream.

But for the "lengthy" part, this is a few lines and characters shorter, albeit C++11:

cout << "   a b c d e f g h i j\n"
        "  +-------------------+\n";
for (int i = 0; i < 10; i++) {
    cout << ' ' << i << '|';
    for (char s : score[i])
      cout << s;
    cout << "         |\n";
}
cout << "  +-------------------+\n";

I don't understand why the spaces should be printed at the end instead of aligning the inner columns nicely, but I followed what you did in your code.

I have also got rid of the unnecessary endl which triggers stream flush and changed single letter strings to character constants, but I'm a bit doubtful of the resulting efficiency gain there. After all, it's just printing some output, not a time-critical computation task.

Yirkha
  • 12,737
  • 5
  • 38
  • 53
0

I'm going to assume that when you said you wanted to make it efficient and not lengthy what you really meant was that you wanted it to be correct and readable.

I don't believe what you currently have is "correct". I assume that char score[10][10] will contain a single printable character for each square of the board and perhaps a null character for cells where you don't want to print anything. And you want to print the contents of score into the template shown. As it stands, if you put anything other than a single space into char score[10][10] you will mess up your template.

As for readability, I think what you currently have is reasonably readable, perhaps it would benefit by having some functions extracted with meaningful names but that is just my personal preference. Based on my assumptions about what you were trying to do here is my corrected and refactored version:

#include <iostream>
#include <vector>
#include <string>

void printHeader() {
  std::cout << "   a b c d e f g h i j\n";
  std::cout << "  +-------------------+\n";
}

void printFooter() {
  std::cout << "  +-------------------+\n";
}

void printRowStart(int i) {
  std::cout << " " << i << "|";
}

void printRowEnd() {
  std::cout << "|\n";
}

void printSquare(char score) {
  char printable_score = (score != '\0') ? score : ' ';
  std::cout << printable_score;
}

void printRowScore(char (&row_score)[10]) {
  printSquare(row_score[0]);
  for (int i = 1; i != 10; ++i) {
    std::cout << " ";
    printSquare(row_score[i]);
  }
}

void printScore(char (&score)[10][10]) {
  printHeader();
  for (int i = 0; i != 10; ++i) {
    printRowStart(i);
    printRowScore(score[i]);
    printRowEnd();
  }
  printFooter();
}

int main(){
  char score[10][10] = { { 0 } };

  // Added to test assumed usage
  score[4][5] = 'x';
  score[4][6] = 'x';

  printScore(score);
}

You might also want to consider making the code print to a generic ostream to make it more testable.

Chris Drew
  • 14,926
  • 3
  • 34
  • 54