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

SVG mosaic creator

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

Problem

Although my code works as expected, there are a few gotchas.

  • A single row is not filled at once; instead, I can see partially filled rows during the rendering process(Fixed in the updated code below.).



  • The application still feels slow; can it be optimized further?



  • And the biggest thing, how can I make the code more OOPsy?



index.html

```





Mosaic

.container {
margin: 0 auto;
width: 50%;
}
.container ul {
list-style: none;
margin-left: 0;
}




var SVG_URL = 'http://localhost:8765/color/';
function httpGet(url) {
return new Promise(function(resolve, reject) {
var req = new XMLHttpRequest();
req.open('GET', url);
req.onload = function() {
if (req.status == 200) {
resolve(req.response);
} else {
reject(Error(req.statusText));
}
};
req.onerror = function() { reject(Error('Network Error')); };
req.send();
});
};

function getSvg(data) {
return httpGet(SVG_URL + data.hex)
.then(function(svg) {
return {svg: svg, x: data.x, y: data.y};
})
.catch(function(error) {
console.log(error);
});
};

function messageHandler(e) {
var chunks = e.data;
Promise.all(chunks.map(function(data) { return getSvg(data); }))
.then(function(response) {
self.postMessage(response);
// Close the woker to be garbage collected.
//self.close();
});
};

self.addEventListener('message', messageHandler, false);








(function(app) {
document.addEventListener('DOMContentLoaded', function

Solution

Obviously this code was posted nearly 3 years ago, so you have likely updated it, and since then ecmascript-6 has become a lot more prevalent/standard, so you could utilize many features/keywords of that specification to streamline this code. You could also use the fetch API or another promise-based XHR library to simplify the XHR promise code.

Your Questions


The application still feels slow; can it be optimized further?

You could consider using for...of or even regular for loops instead of functional iterators.


And the biggest thing, how can I make the code more OOPsy?

You could use the ecmascript-6 class syntax, even though it is just "syntactical sugar over JavaScript's existing prototype-based inheritance"1.

Other feedback/suggestions

Let's look at the forEach loop within renderRow() (defined inside drawMosiac()).

var tiles = [];
rowData[i].forEach(function(data) {
  var tile = new SVGTile(data.svg, data.x, data.y);
  tiles.push(tile);
});


Perhaps you know this already but this code could be simplified using .map().

const tiles = rowData[i].map(data => new SVGTile(data.svg, data.x, data.y));


If you kept the .forEach() then there isn't much point in assigning the value to tile right before it is pushed into the array - a linter should point that out as excess memory utilization. As was mentioned above, the functional code might be one reason why "the application still feels slow", so you might consider using a for loop if necessary.

One micro-optimization might be to move the incrementing of i here:

res.push(new Tile(data.subarray(i * 4, i * 4 + 3), col, row));
    i++;


To have the post-fix increment operator after the last usage of i within the subarray() call:

res.push(new Tile(data.subarray(i * 4, i++ * 4 + 3), col, row));


1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes

Code Snippets

var tiles = [];
rowData[i].forEach(function(data) {
  var tile = new SVGTile(data.svg, data.x, data.y);
  tiles.push(tile);
});
const tiles = rowData[i].map(data => new SVGTile(data.svg, data.x, data.y));
res.push(new Tile(data.subarray(i * 4, i * 4 + 3), col, row));
    i++;
res.push(new Tile(data.subarray(i * 4, i++ * 4 + 3), col, row));

Context

StackExchange Code Review Q#134633, answer score: 2

Revisions (0)

No revisions yet.