2
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ Why char* a[] = {"t","e","s","t”,”\0"}; instead of const char *a = "test"? \$\endgroup\$ – Bailey Parker Aug 17 '18 at 6:44
  • \$\begingroup\$ thank you @BaileyParker I'll run the code with your approach too. \$\endgroup\$ – Yash Bedi Aug 17 '18 at 7:10
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ the 'std' thing can be fixed by adding using namespace std; \$\endgroup\$ – thxou Aug 17 '18 at 11:40
  • \$\begingroup\$ @thxou, it could be, but that's a harmful thing to recommend. See Why is “using namespace std” considered bad practice? for the reasons why I won't advise that. \$\endgroup\$ – Toby Speight Aug 17 '18 at 11:44
  • \$\begingroup\$ I agree, not a good practice, but it fixes the immediate need of just compile the program provided. \$\endgroup\$ – thxou Aug 17 '18 at 11:52
  • 1
    \$\begingroup\$ using std::cout; is shorter and less harmful. :-) \$\endgroup\$ – Toby Speight Aug 17 '18 at 11:57
  • \$\begingroup\$ Bravo.! what an answer this is. Awesomely Explained. Thanks a Ton Mr.Toby. \$\endgroup\$ – Yash Bedi Aug 19 '18 at 11:54
2
\$\begingroup\$

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.)

\$\endgroup\$
  • 3
    \$\begingroup\$ "Define what problem you're trying to solve" is always good advice for interviews. Well-devised interview questions provide openings to demonstrate your requirements-gathering skills as much as your algorithms. Here, we can't tell from a single example whether the width is fixed at 4, or should adapt if we change the string to be display, or should be provided as an input. That's the kind of question to ask before rushing into a solution! \$\endgroup\$ – Toby Speight Aug 17 '18 at 14:46
  • \$\begingroup\$ I have to admit your approach was my first thought as well since the problem description in the OP is severly lacking. I guess programmers always try to weasel their way out of having to do any work. \$\endgroup\$ – yuri Aug 18 '18 at 10:24

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.