Scroll text across a fixed width

Question

In an interview, I was asked to write code to produce this output:

    T
   TE
  TES
 TEST
 EST
 ST
 T

The code I wrote was:

#include<iostream>
int main() {
char* a[] = {"t","e","s","t","\0"};
int k=0, i,j;
for(i=0; i<4; i++) {
    while(k<3) {
        cout<<" ";
        k++;
    }
    k=i+1;
    for(j=0; j<=i; j++) {
        cout<<a[j];
    }
    cout<<"\n";
}
for(i=1; i<4; i++) {
    for(j=i; j<4; j++) {
        cout<<a[j];
    }
    cout<<"\n";
}
return 0;
}

I think the code is not precise also the complexity is really a lot more that what I expected.

How can I improve this code in terms of time and space complexity?

I was wondering if additional sets of eyes could spot potential errors. Also, any stylistic or performance improvement suggestions would be appreciated

Answer 1

Syntax

First things first - I had to fix a handful of syntax errors for this to compile. The most glaring is the misspelling of std::cout in many places. The other bug is the forbidden conversion of string literals from const char* to char* - fixed by declaring a as const char* a[].

Style

The code is very dense - more whitespace around operators will help readability. The one-letter variable names also hinder comprehension - i and j are obviously loop indexes, but what's k for?

These style issues are especially important in interviews - your interviewer wants to know whether you can write code that other employees are able to work with.

Structure

Everything here is crammed into main(). That's not great practice in general, as it makes your code hard to re-use. Prefer to write a function - that's more amenable to unit test, allowing you to specify different strings to scroll, and to capture the output (e.g. into a std::ostringstream).

Algorithm

Rather than writing a single character at a time, it's probably easier to work with a std::string. If we pad both ends with spaces, it's then just a matter of selecting the right substring to print at any given time.

---

Re-writing the code

1. I'd start by defining the interface I want to use. Something like this:

   #include <iostream>
   
   int main()
   {
       scroll_text(std::cout, "TEST", 4);
   }
   

2. Now we know how we want to call it, let's write the signature:

   void scroll_text(std::ostream& out,
                    const std::string& text,
                    unsigned width)
   

3. And now implement using the padding strategy I mentioned:

   const std::string padding(width-1, ' ');
   auto padded_text = padding + text + padding;
   
   for (unsigned i = 0;  i <= padded_text.length() - width;  ++i) {
       out << padded_text.substr(i, width) << '\n';
   }
   

Putting this all together:

#include <ostream>
#include <string>

void scroll_text(std::ostream& out,
                 const std::string& text,
                 unsigned width)
{
    const std::string padding(width-1, ' ');
    auto padded_text = padding + text + padding;

    for (unsigned i = 0;  i <= padded_text.length() - width;  ++i) {
        out << padded_text.substr(i, width) << '\n';
    }
}


// Test program

#include <iostream>

int main()
{
    scroll_text(std::cout, "TEST", 4);
}

---

Advanced

We could make the code more efficient by using std::string_view to reduce the amount of copying. I won't show that here, but you might consider it as an exercise.

Another exercise - make the width argument optional, and if not specified, then use the length of the input string as the width.

Answer 2

For one thing, you should always format your code properly (with indentation) when posting it publicly. Some would say you should even format it properly when writing it, regardless of where you're going to post it. :)

Second, you'll have to define what problem you're trying to solve. Your stated problem is easily solved by something like this:

#include <stdio.h>

int main() {
    puts("   T");
    puts("  TE");
    puts(" TES");
    puts("TEST");
    puts("EST ");
    puts("ST  ");
    puts("T   ");
}

Much shorter, and it can be seen at a glance to be correct! (Your posted solution works, modulo case-sensitivity, but it is long and not obviously correct.)