HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Drawing a picture of some trees using JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
picturejavascripttreesdrawingusingsome

Problem

I want to know how I can improve this piece of JS with respect to best practices/performance.

JS

```
var treeGroupTypes, treeType, leftLeafClass, rightLeafClass;
treeGroupTypes = ["tree-group-small", "tree-group-avg", "tree-group-large", "tree-group-large", "tree-group-avg", "tree-group-small", "tree-group-small", "tree-group-avg", "tree-group-large", "tree-group-large", "tree-group-avg", "tree-group-small"];
treeType = ["small-tree", "avg-tree", "large-tree"];
leftLeafClass = "left-leaf";
rightLeafClass = "right-leaf";

//Both the above arrays have css classes in them, and then the 2 variables as well. Basically the whole js codes builds some trees and appends leaves to them.

buildTrees(treeGroupTypes, treeType, leftLeafClass, rightLeafClass);

buildTrees: function (treeGroupTypes, treeType, leftLeafClass, rightLeafClass) {
for (j = 0; j < treeGroupTypes.length; j++) {
var treeGroup;
treeGroup = $(document.createElement("div"));
treeGroup.addClass(treeGroupTypes[j]).appendTo(".trees")

for (i = 0; i < treeType.length; i++) {
var leftLeaf, rightLeaf, leftIcon, rightIcon;
leftLeaf = $(document.createElement("span"));
rightLeaf = leftLeaf.clone();
leftIcon = $(document.createElement("i"));
rightIcon = leftIcon.clone();
leftLeaf.addClass(leftLeafClass).append(leftIcon);
rightLeaf.addClass(rightLeafClass).append(rightIcon);

var tree = $(document.createElement("div"));
tree.addClass(treeType[i]).appendTo(treeGroup);
leftLeaf.appendTo(tree);

if (treeGroupTypes[j] == "tree-group-large" && treeType[i] == "large-tree") {
for (l = 1; l < 4; l++) {
var more = rightLeaf.clone();
more.css("top", (tree.height() / 4) * l).appendTo(tree)
}
}

Solution

I am using color codes for clarity of my explanation, not suggesting you use them in the code.

As the image is static. Just use a set code.

You have 5 tree heights:

red, green, yellow, blue (=orange), purple

Tree.size 1 2 3 4 5


You have 5 left leaf, 4 right leaf

leaf.left 1 2 3 4 5   
leaf.right 1 2 3 4 

red tree = .size1 +  leaf.left1 + leaf.right1.
green tree = .size2 + leaf.left2 + leaf.right2  //..etc


Using the classes in css.

red tree
 green tree
 yellow tree
 space
 green tree
 yellow tree
 blue tree
 space //..etc


You know how to code, just use the above algorithm.

To change the width of the trunks, just add change to format accordingly.

Honestly, your ability to loop shows you have a talent, and put a lot of thought into your code. But in this case, where you are producing a static image, it is over complicated and unnecessary. Save your code for situations, where you require user input; I am sure it could be modified to be useful for a more dynamic website.

You can achieve the same result using css and no js. I think the use of js is an overkill. With websites, if you can achieve the same result will good css, I think (my opinion) that it is the better way to go. Back in the day js was treated suspiciously and often disabled on web browser, but this is less so the case now. However, I would recommend, that if you can achieve the same result using css, I wouldn't code such a thing using js.

Personally, I would reserve js for client side validation, or interaction in a dynamic website. Not for producing a static image.

My opinion.

Edit

Also, you have a lot of nested looping going on.. and I haven't addressed that in this answer, I am sure that could be optimised.

Code Snippets

Tree.size 1 2 3 4 5
leaf.left 1 2 3 4 5   
leaf.right 1 2 3 4 

red tree = .size1 +  leaf.left1 + leaf.right1.
green tree = .size2 + leaf.left2 + leaf.right2  //..etc
red tree
 green tree
 yellow tree
 space
 green tree
 yellow tree
 blue tree
 space //..etc

Context

StackExchange Code Review Q#31892, answer score: 2

Revisions (0)

No revisions yet.