Inspired by Nested for-loop ASCII art, I looked at building a more complicated ASCII art than just the normal 'diamond' and 'triangle' variations that crop up occasionally here.
The challenge: Produce the following rocket
/**\
//**\\
///**\\\
////**\\\\
/////**\\\\\
+=*=*=*=*=*=*+
|../\..../\..|
|./\/\../\/\.|
|/\/\/\/\/\/\|
|\/\/\/\/\/\/|
|.\/\/..\/\/.|
|..\/....\/..|
+=*=*=*=*=*=*+
|\/\/\/\/\/\/|
|.\/\/..\/\/.|
|..\/....\/..|
|../\..../\..|
|./\/\../\/\.|
|/\/\/\/\/\/\|
+=*=*=*=*=*=*+
/**\
//**\\
///**\\\
////**\\\\
/////**\\\\\
The question on Stack Overflow has issues, and those issues got answered before I could put a response together, but, the problem intrigued me, and I built the following code to solve it in a more compact way, using loops as much as possible, and parameterizing almost all aspects of the problem.
public class Rocket {
private enum Direction {
FORWARD, BACKWARD
}
private static final char NONE = (char) 0;
private static final String buildSymmetrical(
char[] chars, char mid, int from, int len) {
StringBuilder sb = new StringBuilder(len * 2 + 2);
for (int i = 0; i < len; i++) {
sb.append(chars[from + i]);
}
if (mid != NONE) {
sb.append(mid).append(mid);
}
for (int i = 0; i < len; i++) {
sb.append(chars[chars.length - from - len + i]);
}
return sb.toString();
}
private static final String buildLines(
char bracket, char[] source, int len, char midchar, int charfrom,
Direction chardir, int linecount, int duplicates) {
StringBuilder sb = new StringBuilder();
for (int line = 0; line < linecount; line++) {
sb.append(bracket);
int pos = charfrom + line * (chardir == Direction.FORWARD ? 1 : -1);
String block = buildSymmetrical(source, midchar, pos, len);
for (int d = 0; d < duplicates; d++) {
sb.append(block);
}
sb.append(bracket).append('\n');
}
return sb.toString();
}
public static final String buildRocket() {
char[] wider = "../\\/\\/\\/\\..".toCharArray();
char[] narrower = "..\\/\\/\\/\\/..".toCharArray();
char[] cone = " //////\\\\\\\\\\\\ ".toCharArray();
char[] boundary = "+=*=*=*=*=*=*+\n".toCharArray();
StringBuilder sb = new StringBuilder();
sb.append(buildLines(' ', cone, 5, '*', 0, Direction.FORWARD, 5, 1));
sb.append(boundary);
sb.append(buildLines('|', wider, 3, NONE, 0, Direction.FORWARD, 3, 2));
sb.append(buildLines('|', narrower, 3, NONE, 2, Direction.BACKWARD, 3, 2));
sb.append(boundary);
sb.append(buildLines('|', narrower, 3, NONE, 2, Direction.BACKWARD, 3, 2));
sb.append(buildLines('|', wider, 3, NONE, 0, Direction.FORWARD, 3, 2));
sb.append(boundary);
sb.append(buildLines(' ', cone, 5, '*', 0, Direction.FORWARD, 5, 1));
return sb.toString();
}
public static void main(String[] args) {
System.out.println(buildRocket());
}
}
My particular concerns are the number of parameters I have going to the inner methods. How can those be rationalized? Should they be? What mechanisms can be used to simplify the interfaces?
-
2\$\begingroup\$ And if you want the worst code possible, instead... ;) \$\endgroup\$Doorknob– Doorknob2014年10月08日 21:07:13 +00:00Commented Oct 8, 2014 at 21:07
2 Answers 2
Wells, for the sake of loop-ifying everything, you may want to consider changing boundary
as such:
final char[] boundary = "=*=*=*".toCharArray();
...
sb.append( buildLines( '+', boundary, 3, NONE, 0, Direction.CENTER, 1, 2 ) );
That will create =*=*=*
, duplicate it once, and add the +
character at both ends.
The ternary operator below looks like a good candidate for adding a helper method into the Direction
enum
:
int pos = charfrom + line * (chardir == Direction.FORWARD ? 1 : -1)
My version is expanded as such:
private enum Direction {
CENTER( 0 ), UPWARD( 1 ), DOWNWARD( -1 );
private final int multiplier;
Direction( final int multiplier ) {
this.multiplier = multiplier;
}
int getMultiplier() {
return multiplier;
}
}
And calculating pos
is just charfrom + line * chardir.getMultiplier()
;
One more minor suggestion: The usage of 5
and 3
seems to be repeated quite often, maybe refer to them as coneSize
and blockSize
? The main body will look something like:
sb.append( buildLines( ' ', cone, coneSize, '*', 0, Direction.UPWARD, coneSize, 1 ) );
sb.append( buildLines( '+', boundary, blockSize, NONE, 0, Direction.CENTER, 1, 2 ) );
sb.append( buildLines( '|', wider, blockSize, NONE, 0, Direction.UPWARD, blockSize, 2 ) );
sb.append( buildLines( '|', narrower, blockSize, NONE, 2, Direction.DOWNWARD, blockSize, 2 ) );
sb.append( buildLines( '+', boundary, blockSize, NONE, 0, Direction.CENTER, 1, 2 ) );
sb.append( buildLines( '|', narrower, blockSize, NONE, 2, Direction.DOWNWARD, blockSize, 2 ) );
sb.append( buildLines( '|', wider, blockSize, NONE, 0, Direction.UPWARD, blockSize, 2 ) );
sb.append( buildLines( '+', boundary, blockSize, NONE, 0, Direction.CENTER, 1, 2 ) );
sb.append( buildLines( ' ', cone, coneSize, '*', 0, Direction.UPWARD, coneSize, 1 ) );
Due to the fact that our sources
are fixed lengths now, I don't see much of an improvement on the method signatures since it's not like the current code can be easily expanded to create rockets of different sizes. Maybe I'll attempt another stab at that when I have some more time...
-
2\$\begingroup\$ Neat, I never thought of putting the boundary through the buildLines loop. It does make the process more consistent. The expanded enum is a tradeoff though, I am on the fence with that one. My first version of the code sent the multiplier in as an int, I think the enum is better, but embedding the multiplier in the enum seems to be too much. The rest of your post is great. I look forward to seeing your attempt. \$\endgroup\$rolfl– rolfl2014年10月08日 04:20:25 +00:00Commented Oct 8, 2014 at 4:20
My particular concerns are the number of parameters I have going to the inner methods. How can those be rationalized?
That's a tough nut, the only idea I have is to aggregate them somehow. I couldn't find any strong relationship between the parameters, e.g., like that the two first always work together, so I don't know.
Should they be?
Probably yes... although for a private method we could be tolerant.
char[] wider = "../\\/\\/\\/\\..".toCharArray();
Are you optimizing? What's wrong with a String? It's immutable, so passing it is better than passing an array.
private static final char NONE = (char) 0;
Why don't you simply use a String instead of char midchar
?
int charfrom
This looks like a char
, but is none. What about simply from
or start
or whatever?
buildSymmetrical
is a bit too complicated. There are
append(char[] str, int offset, int len)
and
append(CharSequence s, int start, int end)
which would together with StringBuilder#reverse
save you the loops.
I'd actually go for a different approach, namely drawing. Something like
draw(String what, int row, int column, int repeatCount)
together with some AffineTransform
would be very flexible. I'd draw just a left half and then mirror it.
This would be most probably way slower than your solution, but in case you want a really really fast one, I'd suggest
return ""
+ " /**\\\n"
+ " //**\\\\\n"
+ " ///**\\\\\\\n"
...
+ " ////**\\\\\\\\\n"
+ " /////**\\\\\\\\\\\n"
+ "";
An additional advantage is that it's 26 lines only. Am I cheating?
Cool question, btw.
-
6\$\begingroup\$ That last recommendation...( ͡° ͜ʖ ͡°) \$\endgroup\$Quaxton Hale– Quaxton Hale2014年10月08日 04:41:42 +00:00Commented Oct 8, 2014 at 4:41
-
3
-
\$\begingroup\$ @Doorknob Then post it here and let's review it. It looks like you could improve the spacing a bit. ;) \$\endgroup\$maaartinus– maaartinus2014年10月09日 00:50:44 +00:00Commented Oct 9, 2014 at 0:50
Explore related questions
See similar questions with these tags.