23
\$\begingroup\$

My task:

Write nested for loops to produce the following output with each line 48 characters wide:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~
+~++~++~++~++~++~++~++~++~++~++~++~++~++~++~++~+     
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is my idea, but I guess it's pretty much redundant. If so, how can I minimize redundancy here? My output is correct, by the way.

public class Exercises {
    public static void main (String[] args){

    for (int i = 1; i <= 4; i++){
        for (int j = 1; j <= 12; j++){
            System.out.print("~");
        }
    }
            System.out.println();
            System.out.print("~");

    for (int i = 1; i <=15; i++){
            System.out.print("+~~");
    }
            System.out.println("+~");
            System.out.print("+~");
    for (int i = 1; i <= 15; i++){
            System.out.print("++~");
        }
            System.out.println("+");

    for (int i = 1; i <= 4; i++){
        for (int j = 1; j <= 12; j++){
            System.out.print("~");
        }
    }
  }
}
\$\endgroup\$
  • 9
    \$\begingroup\$ Your indenting is really weird. \$\endgroup\$ – Fund Monica's Lawsuit Nov 27 '15 at 2:55
20
\$\begingroup\$
for (int i = 1; i <= 4; i++){
    for (int j = 1; j <= 12; j++){
        System.out.print("~");
    }
}
System.out.println();

can easily be:

for (int i = 0; i < 48; i++) {
    System.out.print('~');
}
System.out.println();

Some edits:

  • Improved formatting: ){ to ) {
  • Simplified to one for loop that runs from 0 to 47 (start at 0, run to less than 48)
  • Print a single char instead of a String with one char in it to improve performance

This:

        System.out.print("~");

for (int i = 1; i <=15; i++){
        System.out.print("+~~");
}
        System.out.println("+~");

Can be this:

for (int i = 0; i < 16; i++){
    System.out.print("~+~");
}
System.out.println();

What changed:

  • Better formatting
  • Loops and prints the ~+~ pattern
        System.out.print("+~");
for (int i = 1; i <= 15; i++){
        System.out.print("++~");
    }
        System.out.println("+");

to:

for (int i = 0; i < 16; i++){
    System.out.print("+~+");
}
System.out.println();

Changes made:

  • Better formatting
  • Loops and prints the +~+ pattern

All of the fixes have some things in common:

  • Indentation and formatting
  • Find the correct pattern

Other changes:

  • Use methods:

    Currently all your code is in the main method. Split it up.

Final code:

public class Exercises {

    public static void main (String[] args){
        printTildes();
        printTPT_Pattern();
        printPTP_Pattern();
        printTildes();
    }

    private static void printTildes() {
        for (int i = 0; i < 48; i++){
            System.out.print("~");
        }
        System.out.println();
    }

    private static void printTPT_Pattern() {
        for (int i = 0; i < 16; i++){
            System.out.print("~+~");
        }
        System.out.println();
    }

    private static void printPTP_Pattern() {
        for (int i = 0; i < 16; i++){
            System.out.print("+~+");
        }
        System.out.println();
    }

}
\$\endgroup\$
  • 7
    \$\begingroup\$ "Print a single char…to improve performance"—please stop encouraging micro-optimizations. Suggesting "improvements" like this leads people to believe that premature optimization is a good idea. In this case, I'd argue that the consistency with the rest of the code is far more important. And, in fact, print(char) delegates to print(String) and not the other way around, at least in OpenJDK, so the negligible performance gain is in the other direction. \$\endgroup\$ – wchargin Nov 26 '15 at 22:20
  • 11
    \$\begingroup\$ This answet got accepted, but it does not meet the task requirements. It's been explicitly requested to 'write nested for loops' to produce the given output, and the answer presents no nested loops. -1 \$\endgroup\$ – CiaPan Nov 27 '15 at 7:34
  • 7
    \$\begingroup\$ The 3 methods are very close and could be refactored into one... \$\endgroup\$ – assylias Nov 27 '15 at 8:23
41
\$\begingroup\$

You're not breaking up the patterns in the most logical way. Each line just consists of repeating blocks of three characters.

public class SquigglePlus {
    private static final String[] PATTERNS = { "~~~", "~+~", "+~+", "~~~" };

    public static void main(String[] args) {
        for (int line = 0; line < PATTERNS.length; line++) {
            for (int col = 0; col < 48; col += PATTERNS[line].length()) {
                System.out.print(PATTERNS[line]);
            }
            System.out.println();
        }
    }
}

Alternatively, the inner loop could just count to 16: for (int i = 0; i < 16; i++) { … }. Note that in Java, that is the most idiomatic way to count: start with 0, and use < for the termination check. The other way (for (int i = 1; i <= 16; i++) { … }) would not be wrong, but usually you would write it that way only if you had a special reason.

Better yet, use a modern enhanced for-loop instead of the old-style counting loop.

public class SquigglePlus {
    private static final String[] PATTERNS = { "~~~", "~+~", "+~+", "~~~" };

    public static void main(String[] args) {
        for (String pattern : PATTERNS) {
            for (int col = 0; col < 48; col += pattern.length()) {
                System.out.print(pattern);
            }
            System.out.println();
        }
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ Since you explicitly count characters printed out, you don't need to keep patterns equal lengths – you can shrink each of them to the shortest period: PATTERNS = { "~", "~+~", "+~+", "~" }; \$\endgroup\$ – CiaPan Nov 27 '15 at 7:30
  • 1
    \$\begingroup\$ A less common counting for approach is with counting downwards: for (int charstowrite = 48; charstowrite > 0; charstowrite -= pattern.length()) It seems to offer no gain here, but if you get some length variable instead of a 48 constant, it allows the loop condition expression to compare a counting variable against a constant 0 instead of comparing two variables. \$\endgroup\$ – CiaPan Nov 27 '15 at 12:15
12
\$\begingroup\$

General things

You have two loops like this:

for (int i = 1; i <= 4; i++){
    for (int j = 1; j <= 12; j++){
        System.out.print("~");
    }
}

This is just nesting a loop for the sake of nesting a loop. I understand that the requirements state to use nested loops, but you should not make code purposefully inefficient just for the sake of it. This should just be for (int i = 1; i <= 48; i++)


You are doing System.out.print() a lot. Your code would be more clear if you used something like a StringBuilder, and append characters to it, and only print it once at the end after it is completed:

class Exercises {
    public static void main (String[] args){

        StringBuilder output = new StringBuilder("");

        for (int i = 1; i <= 48; i++){
            output.append("~");
        }
        output.append("\n");
        output.append("~");

    //etc.

        System.out.print(output);
    }
  }
}

Approach

The approach you have is not ideal, as has been addressed by others answers. I will take a grounds-up approach as to how I think would be a good way to approach a problem like this.

There are some things we know about the required pattern that we can declare as constants, as they will never change.

  • The length of the lines (48)
  • The number of lines (4)
  • That each line has its own pattern (the pattern itself is not very important)

And some things we can infer, by doing basic armchair math and just looking at the lines.

  • 48 is divisible by 2, 3, 4, 8, 12, 16, etc.
  • Necessarily a pattern is going to be one of those, if we are going to divide the line evenly
  • By looking at each line more closely, it looks like the smallest observable pattern is 3.

So, let's declare all this at the top:

    final int NUMBER_OF_LINES = 4;
    final int LINE_LENGTH = 48;
    final int PATTERN_LENGTH = 3;
    final String[] LINE_NUMBER_PATTERNS = {"~~~","~+~","+~+","~~~"};

So we know how long each line is, and how many times to iterate each pattern, and that we will need to first iterate over each line, and then iterate over the characters per line. So, the skeleton might look like this:

    for (int currentLine = 1; currentLine <= NUMBER_OF_LINES; currentLine++) {    
        for (int i = 1; i <= LINE_LENGTH; i += PATTERN_LENGTH) {
            // do some stuff here
        }
    }

You also noticed I made a String[] which is an array of strings containing a pattern for each line. The patterns could be any string of characters, and of any length. Arrays are 0-indexed, so we will use 0 to 3 to count the lines, instead of 1 to 4. (notice the use of < instead of <=, as we want to stop at index 3 which is the 4th line)

    for (int currentLine = 0; currentLine < NUMBER_OF_LINES; currentLine++) {    
        for (int i = 1; i <= LINE_LENGTH; i += PATTERN_LENGTH) {
            output.append(LINE_NUMBER_PATTERNS[currentLine]);
        }
    }

It outputs this, which is almost correct:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~~+~+~++~++~++~++~++~++~++~++~++~++~++~++~++~++~++~+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So, all we are missing is a line break "\n" at the end of each line, after we exit the inner loop. So everything all said and done, this is what we have, and it works perfectly. (demo on ideone.com)

class Main {
    public static void main (String[] args){

        final int NUMBER_OF_LINES = 4;
        final int LINE_LENGTH = 48;
        final int PATTERN_LENGTH = 3;
        final String[] LINE_NUMBER_PATTERNS = {"~~~","~+~","+~+","~~~"};

        StringBuilder output = new StringBuilder("");

        int index;
        for (int currentLine = 1; currentLine <= NUMBER_OF_LINES; currentLine++) {    
            index = currentLine -1;
            for (int i = 1; i <= LINE_LENGTH; i += PATTERN_LENGTH) {

                output.append(LINE_NUMBER_PATTERNS[index]);
            }
            output.append("\n");
        }
        System.out.print(output);
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ How about using for (int currentLine = 0; currentLine < NUMBER_OF_LINES; currentLine++) { instead? \$\endgroup\$ – Simon Forsberg Nov 26 '15 at 19:08
  • 2
    \$\begingroup\$ Why use NUMBER_OF_LINES when you have LINE_NUMBER_PATTERNS.length ? \$\endgroup\$ – Simon Forsberg Nov 26 '15 at 19:09
  • 1
    \$\begingroup\$ First awesome answer. Just two minor nits. (1) having the main function do everything makes it harder to maintain or modify. A function that returns the stringbuilder would be better. (2) the prints are a problem, because they couple what you want to do to outputting the results. For example, the first and last patterns/lines are the same, printing out the intermediate results makes it harder to take advantage of that fact. \$\endgroup\$ – jmoreno Nov 28 '15 at 5:52
  • \$\begingroup\$ @jmoreno I completely agree, I hope you will understand that I approached this "as a beginner" to try and match the spirit of the question, though I certainly learned from it :) \$\endgroup\$ – Phrancis Nov 28 '15 at 7:47
  • \$\begingroup\$ @Phrancis: Sure I understand that, and the 2nd point was my main one (insofar as minor nitpicking can have a "main" point) and was intended to address your "nothing wrong per-se". I think there is something wrong with mixing computting a value and outputting the result. I was/would suggest deleteing everthing from "While" to ", but ". \$\endgroup\$ – jmoreno Nov 28 '15 at 16:14
10
\$\begingroup\$

Patterns. Aren't they lovely?

There are two things I don't like in your code:

  • You're using a lot of repeated System.out.print calls which does have some overhead (might be considered premature optimization, but I think it is good to know about this)
  • You are not making as much use of the patterns as you could be.

Let's take a look at your current code:

System.out.print("~");
for (int i = 1; i <=15; i++){
    System.out.print("+~~");
}
System.out.println("+~");


System.out.print("+~");
for (int i = 1; i <= 15; i++){
    System.out.print("++~");
}
System.out.println("+");

There's a quite interesting split you have made here. If you look at the patterns you can see that the first is really just a line of ~+~, and the second is +~+.

While your requirement might be to use nested for-loops, I don't think of it as the best way, and as an educational exercise, I will explain how I would do it in a - what I consider - better way.

I would create a method that returns a String of a specific length based on a pattern.

public static String createPattern(String pattern, int length) {
    if (pattern.isEmpty()) {
        throw new IllegalArgumentException("Pattern cannot be empty");
    }
    StringBuilder builder = new StringBuilder(length);
    while (builder.length() < length) {
        int strLength = Math.min(pattern.length(), length - builder.length());
        builder.append(pattern, 0, strLength);
    }
    return builder.toString();
}

Then you could use this method:

String cleanLine = createPattern("~~~", 48);
System.out.println(cleanLine);
System.out.println(createPattern("~+~", 48));
System.out.println(createPattern("+~+", 48));
System.out.println(cleanLine);

Or, you could keep considering your patterns as series of "+~~" that has been shifted one step to the right, this could be done by using a shift method.

public static String shift(String original, int stepsRight) {
    if (Math.abs(stepsRight) >= original.length()) {
        throw new IllegalArgumentException("Steps to shift, in either direction, must be less than string length");
    }
    if (stepsRight < 0) {
        // shifting left one step is the same as shifting it right (length - 1) steps.
        stepsRight += original.length();
    }
    int split = original.length() - stepsRight;
    return original.substring(split) + original.substring(0, split);
}

Then you could do this:

String cleanLine = createPattern("~~~", 48);
System.out.println(cleanLine);
System.out.println(createPattern(shift("+~~", 1), 48));
System.out.println(createPattern(shift("++~", -1), 48));
System.out.println(cleanLine);

I'd go with the non-shifting pattern version though, but shifted patterns could be useful if you would want to make multiple rows with the same pattern, but shifted.

\$\endgroup\$
  • 1
    \$\begingroup\$ There is nesting in some sense, but it's not a nested for-loop as required by the exercise. \$\endgroup\$ – 200_success Nov 26 '15 at 18:44
  • 1
    \$\begingroup\$ @200_success Nested, hmm... did not pay that much attention to that requirement apparently. Could be a slight misinterpretation of the requirements though, I find nested for-loops to not be the best job for this. Added a comment about that in my answer. \$\endgroup\$ – Simon Forsberg Nov 26 '15 at 19:00
7
\$\begingroup\$

The thing you need to do to produce better code is to learn to identify pattern in your code to see if you're duplicating functionality and code. So the first thing I did when I looked at your code was asking myself, is there duplication somewhere? The answer is yes there is!

for (int i = 1; i <= 4; i++){
        for (int j = 1; j <= 12; j++){
            System.out.print("~");
        }
    }

This piece of code is both at the start and the end of your method. So you can extract this in a method on his own and you don't have to duplicate this anymore!

The rest of your code as is "correct" since it does not have duplication but it still not the good way to look at the problem. The task is trying to make think about the pattern you have to outup. You should have see that the second line is the repition of the sequence ~+~ and the third line is the repetition of +~+. I suggest that you try to come with the solution to output with this in mind.

\$\endgroup\$
7
\$\begingroup\$

Here's a Java 8 Stream-based take on this... :)

Using @200_success's answer as a base, we know the repeating groups can be just "~~~", "~+~", "+~+", "~~~" (or following @CiaPan's comment, just "~" since we 'loop' by comparing the length). We also know we need to repeat them 48 / length of each String times.

private static final int LENGTH = 48;

// Usage
// Original
// Stream.of("~~~", "~+~", "+~+", "~~~")
Stream.of("~", "~+~", "+~+", "~")
        .map(v -> String.join("", Collections.nCopies(LENGTH / v.length(), v)))
        .forEach(System.out::println);
  1. Construct a Stream of the desired String elements.
  2. For each element, map() them to the desired result, by join()-ing a List of each element created with nCopies(). This allows us to easily create duplicate elements for the given number of copies, LENGTH / v.length().
  3. Finally, forEach() of the elements, pass it to System.out.println(String) (used as a method reference here).

Of course, one can simply substitute LENGTH / v.length() with a hard-coded 16 (if we are using "~~~" too), but I felt this has more flexibility for future variation (just need to be careful about rounding off for non-factors of LENGTH).

edit: It is relatively trivial to convert that into an explicit for-loop structure, to better fit your task requirements:

// Original
// for (String element : Arrays.asList("~~~", "~+~", "+~+", "~~~")) {
for (String element : Arrays.asList("~", "~+~", "+~+", "~")) {
    StringBuilder builder = new StringBuilder();
    for (int i = 0; i < LENGTH / element.length(); i++) {
        builder.append(element);
    }
    System.out.println(builder.toString());
}

Similar steps are being done here: We loop through the same elements, we 'map' each of them by using a StringBuilder to concatenate an element repeatedly for the required number of copies, and finally, we print the toString() representation of each StringBuilder.

\$\endgroup\$
3
\$\begingroup\$

The key part of this question, that no-one has really answered, is;

Write nested forloops

My first thought when approaching this is to just keep it simple;

public class Exercises
{
    public static void main (String[] args)
    {
        for (int row = 0; row < 4; row++)
        {
            // Loop 0 - 15.
            // "column" 0 - "column" 15 are each 3 chars wide, for a total of 48 chars.
            for (int column = 0; column < 16; column++)
            {
                switch (row)
                {
                    // First and last row are the same, don't repeat code!
                    case 0:
                    case 3:
                        System.out.print("~~~");
                        break;
                    case 1:
                        System.out.print("~+~");
                        break;
                    case 2:
                        System.out.print("+~+");
                        break;
                }
            }
            // There's a newline at the end of each row, print it here, once the "column" printing is done.
            System.out.print("\n");
        }
    }
}

I wouldn't say it's fantastic, but it covers the point that your teacher/tutor/self-help guide is trying to make.

  • We're using nested loops. This one is key.
  • We're using properly named variables row and column. This makes it much easier to understand what's happening within the code.

There's still other things you could do;

  • The entire code could be pulled out of main and into it's own method; WritePattern(). This makes the code more extensible; say the next lesson it to repeat said pattern, all you have to do is add in a parameter to the method and multiply 4 by the parameter.
  • Each case statement could go in separate methods too, perhaps something like WriteRowPattern(int columnLimit, string pattern) where 47/16 is replaced by the columnLimit parameter. Again, this makes it more extensible. With this change, we go from a fixed pattern to whatever pattern we want.
\$\endgroup\$
  • 4
    \$\begingroup\$ I think we have different definitions of 'simple'? \$\endgroup\$ – h.j.k. Nov 27 '15 at 1:27
  • 1
    \$\begingroup\$ Agreed. I don't think I'd be teaching a beginner about anonymous functions, maps and "foreach" loop declarations. Especially since the focus of this question is nested for loops. I think it's fairly easy to understand how the first loop is for each row, the second for each column. You could probably adjust the switch statement and make it a bit clearer, that's about it though. Happy to explain things further if you don't understand a particular part. \$\endgroup\$ – Trent Nov 27 '15 at 5:29
  • \$\begingroup\$ Wells, your solution is missing a character in the first and last lines, as you are printing \n as the 48th character, when that really should be the 49th. Also, explicitly skipping output for column values 17...47 for the second and third lines is... quirky. Having to use 0...4 to loop through each row, and then relying on a switch to determine what should be printed at each row, is also slightly cumbersome, IMHO. Between having to specify "~~~" twice and introducing an index + switch, I think the former is both shorter and easier to implement. \$\endgroup\$ – h.j.k. Nov 27 '15 at 5:49
  • \$\begingroup\$ Yeah Ok, I didn't have a java compiler to check the output, silly oversight on my part, but it's a trivial fix. Think back to when you first learnt java. Referencing the outer for loop's variable in the inner for loop is one of the very first things you get taught and I'm pretty sure that's the concept the OP's teacher/tutor/self-help book is trying to get across. \$\endgroup\$ – Trent Nov 27 '15 at 6:07
  • 1
    \$\begingroup\$ @h.j.k. Yeah, I could probably pull out the newline and just put it at the end/outside of the inner for loop, i.e. implicitly write a newline after each line has finished writing it's "columns". I was going with quick/simple, rather than micro optimisations/refactorings. \$\endgroup\$ – Trent Nov 27 '15 at 6:36
2
\$\begingroup\$

I prefer other answers but if you really are forced to use nested loops, I would go for:

    for (int line = 0; line < 4; line++) {
        for (int i = 0; i < 16; i++) {
            int type = line % 3;
            if (type == 2) {
                System.err.print("+~+");
            } else if (type == 1) {
                System.err.print("~+~");
            } else {
                System.err.print("~~~");
            }
        }
        System.err.println();
    }

Or if you think ternary operator is readable enough:

for (int line = 0; line < 4; line++) {
    for (int i = 0; i < 16; i++) {
        int remainder = line % 3;
        System.err.print(remainder == 2 ? "+" : "~");
        System.err.print(remainder == 1 ? "+" : "~");
        System.err.print(remainder == 2 ? "+" : "~");
    }
    System.err.println();
}
\$\endgroup\$

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.