Code Review Asked on December 24, 2021
class ChristmasTree
{
private static void byHand()
{
System.out.println( "Chritsmas tree, yey." );
System.out.println( " *" );
System.out.println( " ***" );
System.out.println( " *****" );
System.out.println( " *******" );
System.out.println( " ********" );
System.out.println( "**********" );
System.out.println( " |" );
System.out.println( " _" );
}
private static void withLoops()
{
int rows = 6;
StringBuilder sb = new StringBuilder();
for( int row = 0; row < rows; ++row )
{
for( int spaces = rows - row; spaces > 0; --spaces )
{
sb.append( " " );
}
for( int stars = 0; stars < 2 * row + 1; ++stars )
{
sb.append( "*" );
}
sb.append( "n" );
}
for( int spaces = 0; spaces < rows; ++spaces )
{
sb.append( " " );
}
sb.append( "|" );
sb.append( "n" );
for( int spaces = 0; spaces < rows; ++spaces )
{
sb.append( " " );
}
sb.append( "_" );
System.out.println( sb.toString() );
}
private static String spaces( int num )
{
StringBuilder sb = new StringBuilder();
for( int spaces = 0; spaces < num; ++spaces )
{
sb.append( " " );
}
return sb.toString();
}
private static String properly( int rows )
{
StringBuilder sb = new StringBuilder();
for( int row = 0; row < rows; ++row )
{
for( int spaces = rows - row; spaces > 0; --spaces )
{
sb.append( " " );
}
for( int stars = 0; stars < 2 * row + 1; ++stars )
{
sb.append( "*" );
}
sb.append( "n" );
}
sb.append( spaces( rows ) );
sb.append( "|" );
sb.append( "n" );
sb.append( spaces( rows ) );
sb.append( "_" );
return sb.toString();
}
public static void main(String[] args)
{
byHand();
withLoops();
// What if the tree needs to be 100 lines high?
// We arrive at the concept of `scalability`.
System.out.println( properly( 16 ) );
}
}
Topics I have considered but decided against:
java.util.Scanner;
spaces()
called stars()
python docstring
Also there’s this.
Nice implementation, easy to understand and efficient. Few suggestions:
"*".repeat()
you can avoid some of the loopsString.format
it's very powerful for arranging the text*
is better to use a single constant or even better an enumIt can actually be solved in one line:
String christmasTree = IntStream.range(0, rows+2).mapToObj(i->String.format("%1$"+rows+"s%2$s%n",i<rows?"*".repeat(i+1):i<rows+1?"|":"_",i<rows?"*".repeat(i):"")).collect(Collectors.joining());
Given rows=4
the string christmasTree
will be:
*
***
*****
*******
|
_
This is a simplified version with a regular for-loop:
public static String christmasTree(int rows) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < rows; i++) {
sb.append(String.format("%1$" + rows + "s%2$s%n", "*".repeat(i + 1), "*".repeat(i)));
}
sb.append(String.format("%1$" + rows + "s%n", "|"));
sb.append(String.format("%1$" + rows + "s", "_"));
return sb.toString();
}
The key methods are .repeat()
(as @Doi9t suggested) and String.format
.
Regarding the line in the for-loop, the parameters of String.format()
are described below:
%1$
means take the first argument, which is "*".repeat(i + 1)
%2$
means take the second argument, which is "*".repeat(i)
s
means convert the argument to a String
%1$" + rows + "s
pads the first string argument to rows
characters%n
represents the line terminatorMore about String.format (which uses a java.util.Formatter
).
This is a version with a configurable ornament:
public static String christmasTree(int rows, Ornament ornament) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < rows; i++) {
sb.append(String.format("%1$" + rows + "s%2$s%n", ornament.getValue().repeat(i + 1), ornament.getValue().repeat(i)));
}
sb.append(String.format("%1$" + rows + "s%n", "|"));
sb.append(String.format("%1$" + rows + "s", "_"));
return sb.toString();
}
The Ornament
enum:
public enum Ornament{
BULBS("o"),
STARS("*");
private String value;
Ornament(String value){
this.value=value;
}
public String getValue() {
return this.value;
}
}
Merry Christmas in advance System.out.println(christmasTree(10,Ornament.BULBS))
o
ooo
ooooo
ooooooo
ooooooooo
ooooooooooo
ooooooooooooo
ooooooooooooooo
ooooooooooooooooo
ooooooooooooooooooo
|
_
Answered by Marc on December 24, 2021
As @Doi9t said, most of your code is about creating strings of repeated characters like the method below:
private static String spaces( int num )
{
StringBuilder sb = new StringBuilder();
for( int spaces = 0; spaces < num; ++spaces )
{
sb.append( " " );
}
return sb.toString();
}
You can use instead the Arrays.fill method to create strings of replicated characters like below:
private static String spaces( int num , char c)
{
char[] arr = new char[num];
Arrays.fill(arr, c);
return new String(arr);
}
About the construction of your tree like this below for nRows = 6
:
Note : n and whitespaces are invisible
* n <-- row = 0
*** n
***** n
******* n
********* n
*********** n <-- row = 5
| n <-- row = 6
_ n <-- row = 7
You are doing it using one StringBuilder
and consecutive append
operations, but you already can calculate the length of the result and where to put characters different from ' '
to minimize operations.
You can check that lines 6, 7, 0 have in common the characteristic to have just one char in the middle, while every line from 1 to 5 can be obtained from the previous one adding one char to the the left and to the right. So if you start from creating a char
tree like below:
private static String generateTree(int nRows) {
final int nColumns = 2 * (nRows + 1);
final int middle = nColumns / 2;
char[] tree = new char[(nRows + 2) * nColumns];
//...other instructions
return new String(tree);
}
After you can calculate every row of your tree putting it in the right position in tree starting from the trunk lines and the peak of your tree:
char[] row = new char[nColumns];
Arrays.fill(row, ' ');
row[middle] = '|';
row[nColumns - 1] = 'n'; //newline will added to every line of the tree.
System.arraycopy(row, 0, tree, nRows * nColumns, nColumns);
row[middle] = '_';
System.arraycopy(row, 0, tree, (nRows + 1) * nColumns, nColumns);
row[middle] = '*';
System.arraycopy(row, 0, tree, 0, nColumns);
After you can calculate the other lines of the tree starting from the peak and add them to obtain the final result:
private static String generateTree(int nRows) {
final int nColumns = 2 * (nRows + 1);
final int middle = nColumns / 2;
char[] tree = new char[(nRows + 2) * nColumns];
char[] row = new char[nColumns];
Arrays.fill(row, ' ');
row[middle] = '|';
row[nColumns - 1] = 'n';
System.arraycopy(row, 0, tree, nRows * nColumns, nColumns);
row[middle] = '_';
System.arraycopy(row, 0, tree, (nRows + 1) * nColumns, nColumns);
row[middle] = '*';
System.arraycopy(row, 0, tree, 0, nColumns);
for (int i = 1; i < nRows; ++i) {
row[middle - i] = '*';
row[middle + i] = '*';
System.arraycopy(row, 0, tree, i * nColumns, nColumns);
}
return new String(tree);
}
Answered by dariosicily on December 24, 2021
I have some suggestions for your code.
Example
"*".repeat(10) // Will give you "**********"
I suggest that you create a class constant for each of the tree's parts (*
, n
, |
) to allow the code to be easier to refactor / make changes in the future, if you need to (i.e., change the star by a plus sign, etc.).
For the tree's parts, I suggest that you use character instead of string, since the character will take less memory than the string (very small amount in this case, but still).
" "
-> ' '
Answered by Doi9t on December 24, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP