Code that prints ASCII bowties to screen given inputs

Question

The goal of this program is to take an input from Stdin and output some ASCII bowties to Stdout. The first integer in the input represents the number of test cases. Each test case contains 2 ints and a char. The first int represents the length in rows of the bowties, the second int the number of bowties, and the third the character to make the bowtie out of.

Input:

2
5 2 #
11 1 !

Output:

#      # #      #
##    ## ##    ##
# #[]# # # #[]# #
##    ## ##    ##
#      # #      #

!            !
!!          !!
! !        ! !
!  !      !  !
!   !    !   !
!    ![]!    !
!   !    !   !
!  !      !  !
! !        ! !
!!          !!
!            !

This program calculates the internal and external spaces and concatenates them plus the material for the bowtie together for the first half. The second half of the bowties are achieved by copying the first half (I couldn't figure out the absolute value function).

I'm a self-taught Java programmer so I don't know much about conventions on when to use void versus return methods and how to structure my code.

import java.util.*;

public class Main
{
    public static void main(String[] args)
    {
        Scanner in = new Scanner(System.in);

        int sets, width, bowties, length, intspaces, extspaces, mid, counter;
        char mat;

        sets = in.nextInt();
        for(int i = 1; i <= sets; i++)
        {
            length = in.nextInt();
            bowties = in.nextInt();
            mat = in.next().charAt(0);
            width = length + 3;
            mid = length/2 + 1;
            counter = 2;
            String[] out = new String[length];


            for(int x = 0; x < mid; x++)
            {
                if(x == 0)
                {
                  intspaces = width - 2;
                  String intspace = space(intspaces, length);
                  out[x] = mat + intspace + mat;
                }
                else if(x != (mid-1))
                {
                  intspaces = width - 2 * (x + 1);
                  extspaces = x - 1;
                  String intspace = space(intspaces, length);
                  String extspace = space(extspaces, length);
                  out[x] = mat + extspace + mat + intspace + mat + extspace + mat;
                }
                else
                {
                  extspaces = x - 1;
                  String extspace = space(extspaces, length);
                  out[x] = mat + extspace + mat + "[]" + mat + extspace + mat;
                }
            }

            for(int x = mid; x < length; x++)
            {
              int count = x - counter;
              out[x] = out[count];
              counter += 2;
            }

            if(bowties != 1)
            {
            stringMulti(out, length, width, bowties);
            }

            for(int x = 0; x < length; x++)
            {
              System.out.println(out[x]);
            }
            System.out.println();

        }   
    }
    public static String space(int spaces, int length)
    {
        StringBuilder sb = new StringBuilder(length - 2);
        for(int i = 1; i <= spaces; i++)
        {
            sb.append(" ");
        }
        return sb.toString();
    }

    public static void stringMulti(String out[], int length, int width, int sets) 
    {
      for(int x = 0; x < length; x++)
      {
        StringBuilder sb = new StringBuilder(width + sets * (width + 1));
        sb.append(out[x]);
        for(int y = 1; y < sets; y++)
        {
       sb.append(" ");
       sb.append(out[x]);
        }
        out[x] = sb.toString();
      }

    }
}

Answer 1

I think the main issue you need to focus first, is to split the input from the processing and maybe even from the output. I'm not sure if you meet the requirements of the task, when you start outputting stuff before completing the input.

If Java was a conscious choice, you might want to think about a more OOP approach. BowtieSet seems a good candidate for a class.

I assume you left away any input validation/user interface by purpose in relation to the task.

In Java you usually define variables not at the beginning of a method, but where you use them. In addition to that, for loops general start with 0 and end <max.

public class Main
{
    public static class BowtieSet
    {
        private final int  length;
        private final int  repeat;
        private final char style;

        public BowtieSet(final int length, final int repeat, final char style)
        {
            this.length = length;
            this.repeat = repeat;
            this.style = style;
        }

        public String getAscii()
        {
            return "TODO:" + length + "-" + repeat + "-" + style;
        }
    }

    public static void main(String[] args)
    {
        Scanner in = new Scanner(System.in);
        final int numberOfBowtieSets = in.nextInt();
        List<BowtieSet> bowties = new ArrayList<>(numberOfBowtieSets);
        for( int i = 0; i < numberOfBowtieSets; i++ )
        {
            int length = in.nextInt();
            int repeat = in.nextInt();
            char style = in.next().charAt(0);
            bowties.add(new BowtieSet(length, repeat, style));
        }

        bowties.forEach(b -> System.out.println(b.getAscii()));
    }
}

If you stay with an procedural style (totally fine for such a task), you could anyway try to split your main method into smaller well names methods.

Nevertheless, after this initial objectifaction or extracting of a method like

public String getAscii(int length, int repeat, char style)

it is far easier to test your code (automatically and manually). You don't need to type stuff anymore and there is nothing on stdout that needs to be read. As a first step you can simply move the content of your outer for loop into such an method (plus some minor adaptations and appending to a StringBuilder instead of printing to system out).

Afterwards it's time to write some unit tests, or at least two for your samples.

public class BowtieSetTest
{
    @Test
    public void sample1()
    {
        assertEquals("...", new BowtieSet(5, 2, '#').getAscii());
    }

    @Test
    public void sample2()
    {
        assertEquals("...", new BowtieSet(11, 1, '!').getAscii());
    }
}

The length parameter in your space() method is "useless". I think in your application you will have no benefit from defining the capacity of the StringBuilder, it just reduces the readability of your code. In case you ever need an other millisecond, come back to this place. Nevertheless, padding strings can also be done with following common and shorter method:

public static String space(int spaces)
{
    if( spaces == 0 ) return "";
    return String.format("%1$" + spaces + "s", "");
}

You can skip the same optimization in your stringMulti method. In addition to that in Java you usually don't pass arrays around the system and modify them. If we make the the method private and keep the issue local, I think we can live with this for the moment. (List as parameter and new list as return values would be more common)

private static void repeatEachLine(String lines[], int repetitions)
{
    for( int x = 0; x < lines.length; x++ )
    {
        StringBuilder sb = new StringBuilder(lines[x]);
        for( int y = 1; y < repetitions; y++ )
        {
            sb.append(" ");
            sb.append(lines[x]);
        }
        lines[x] = sb.toString();
    }
}

It really took me a moment to figure out what the methods stringMulti did. I hope the new method name will help others (or my future self) to be faster at this point.

Let's get back to your main method. You have two for loops, one to the mid and the second for the remaining lines. You already notice that the lower lines are mirrored from the upper lines, so you can also merge the fors and omit your counter

At this point my remaining getAscii method looks like (Disclaimer: I like oneliner, many don't):

public String getAscii(int length, int repeat, char style)
{
    int width = length + 3;
    int mid = length / 2 + 1;
    String[] lines = new String[length];

    for( int x = 0; x < mid; x++ )
    {
        if( x == 0 )
        {
            int intspaces = width - 2;
            String intspace = space(intspaces);
            lines[x] = style + intspace + style;
            lines[length - x - 1] = lines[x];
        }
        else if( x != (mid - 1) )
        {
            int intspaces = width - 2 * (x + 1);
            int extspaces = x - 1;
            String intspace = space(intspaces);
            String extspace = space(extspaces);
            lines[x] = style + extspace + style + intspace + style + extspace + style;
            lines[length - x - 1] = lines[x];
        }
        else
        {
            int extspaces = x - 1;
            String extspace = space(extspaces);
            lines[x] = style + extspace + style + "[]" + style + extspace + style;
        }
    }

    if( repeat != 1 ) repeatEachLine(lines, repeat);
    return Arrays.stream(lines).collect(Collectors.joining("\n"));
}

I think now it is time to extract the content of the for loop to a getAsciiLine(...) method. Add some tests and check if we can simplify the ifs. At least I would inline the int variables. They no longer add any information.

Let me try an other approach here. Instead of building the string character by character, let me take a blank string of the correct length (char[]) and only fill some places with the style character:

void getAsciiLine(final String[] lines, final int currentLine, final int width, final int mid /*...+ style, length if not an object*/)
{
    char[] string = space(width).toCharArray();
    // set left and right border
    string[0] = style;
    string[width - 1] = style;
    // moving to the center
    string[currentLine] = style; //redundant for 0/width-0, but at least no if
    string[width - currentLine - 1] = style;

    final boolean isMiddleLine = currentLine == (mid - 1);
    if( isMiddleLine )
    {
        string[currentLine + 1] = '[';
        string[currentLine + 2] = ']';
    }
    lines[currentLine] = new String(string);
    if( !isMiddleLine ) lines[length - currentLine - 1] = lines[currentLine];
}

Keep in mind that some refactorings are opinion based, so this answer is not the only truth.

---

BowtieSet:

import java.util.Arrays;
import java.util.stream.Collectors;

public class BowtieSet
{
    private final int  length;
    private final int  repeat;
    private final char style;

    public BowtieSet(final int length, final int repeat, final char style)
    {
        this.length = length;
        this.repeat = repeat;
        this.style = style;
    }

    public String getAscii()
    {
        int width = length + 3;
        int mid = length / 2 + 1;
        String[] lines = new String[length];
        for( int x = 0; x < mid; x++ ) getAsciiLine(lines, x, width, mid);
        if( repeat != 1 ) repeatEachLine(lines, repeat);
        return Arrays.stream(lines).collect(Collectors.joining("\n"));
    }

    private void getAsciiLine(final String[] lines, final int currentLine, final int width, final int mid)
    {
        char[] string = space(width).toCharArray();
        // set left and right border
        string[0] = style;
        string[width - 1] = style;
        // moving to the center
        string[currentLine] = style; //redundant for 0/width-0, but at least no if
        string[width - currentLine - 1] = style;

        final boolean isMiddleLine = currentLine == (mid - 1);
        if( isMiddleLine )
        {
            string[currentLine + 1] = '[';
            string[currentLine + 2] = ']';
        }
        lines[currentLine] = new String(string);
        if( !isMiddleLine ) lines[length - currentLine - 1] = lines[currentLine];
    }

    private static String space(int spaces)
    {
        if( spaces == 0 ) return "";
        return String.format("%1$" + spaces + "s", "");
    }

    private static void repeatEachLine(String lines[], int repetitions)
    {
        for( int x = 0; x < lines.length; x++ )
        {
            StringBuilder sb = new StringBuilder(lines[x]);
            for( int y = 1; y < repetitions; y++ )
            {
                sb.append(" ");
                sb.append(lines[x]);
            }
            lines[x] = sb.toString();
        }
    }
}

Main:

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Main
{

    public static void main(String[] args)
    {
        Scanner in = new Scanner(System.in);
        final int numberOfBowtieSets = in.nextInt();
        List<BowtieSet> bowties = new ArrayList<>(numberOfBowtieSets);
        for( int i = 0; i < numberOfBowtieSets; i++ )
        {
            int length = in.nextInt();
            int repeat = in.nextInt();
            char style = in.next().charAt(0);
            bowties.add(new BowtieSet(length, repeat, style));
        }

        bowties.forEach(b -> System.out.println(b.getAscii()));
    }
}

BowtieSetTest:

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class BowtieSetTest
{
    @Test
    public void sample1()
    {
        assertEquals(//
            "#      # #      #\n" +//
                "##    ## ##    ##\n" +//
                "# #[]# # # #[]# #\n" +//
                "##    ## ##    ##\n" +//
                "#      # #      #",//
            new BowtieSet(5, 2, '#').getAscii());
    }

    @Test
    public void sample2()
    {
        assertEquals(//
            "!            !\n" +//
                "!!          !!\n" +//
                "! !        ! !\n" +//
                "!  !      !  !\n" +//
                "!   !    !   !\n" +//
                "!    ![]!    !\n" +//
                "!   !    !   !\n" +//
                "!  !      !  !\n" +//
                "! !        ! !\n" +//
                "!!          !!\n" +//
                "!            !",//
            new BowtieSet(11, 1, '!').getAscii());
    }
}

The empty comments in the test are only used to trick my IntelliJ IDEA formatter.