Question

As part of a word cloud rendering algorithm (inspired by this question), I created a Javascript / Processing.js function that moves a rectangle of a word along an ever increasing spiral, until there is no collision anymore with previously placed words. It works, yet I'm uncomfortable with the code quality.

So my question is: How can I restructure this code to be:

  • readable + understandable
  • fast (not doing useless calculations)
  • elegant (using few lines of code)

I would also appreciate any hints to best practices for programming with a lot of calculations.

Rectangle moveWordRect(wordRect){
    // Perform a spiral movement from center
    // using the archimedean spiral and polar coordinates
    // equation: r = a + b * phi

    // Calculate mid of rect
    var midX = wordRect.x1 + (wordRect.x2 - wordRect.x1)/2.0;
    var midY = wordRect.y1 + (wordRect.y2 - wordRect.y1)/2.0;

    // Calculate radius from center 
    var r = sqrt(sq(midX - width/2.0) + sq(midY - height/2.0));

    // Set a fixed spiral width: Distance between successive turns
    var b = 15; 

    // Determine current angle on spiral
    var phi = r / b * 2.0 * PI;

    // Increase that angle and calculate new radius
    phi += 0.2;
    r = (b * phi) / (2.0 * PI);

    // Convert back to cartesian coordinates
    var newMidX = r * cos(phi);
    var newMidY = r * sin(phi);

    // Shift back respective to mid
    newMidX += width/2;
    newMidY += height/2;

    // Calculate movement 
    var moveX = newMidX - midX;
    var moveY = newMidY - midY;

    // Apply movement
    wordRect.x1 += moveX;
    wordRect.x2 += moveX;
    wordRect.y1 += moveY;
    wordRect.y2 += moveY;

    return wordRect;
}
Was it helpful?

Solution

The quality of the underlying geometric algorithm is outside my area of expertise. However, on the quality of the code, I would say you could extract a lot of functions from it. Many of the lines that you have commented could be turned into separate functions, for example:

  • Calculate Midpoint of Rectangle
  • Calculate Radius
  • Determine Current Angle
  • Convert Polar to Cartesian Coodinates

You could consider using more descriptive variable names too. 'b' and 'r' require looking back up the code to see what they are for, but 'spiralWidth' and 'radius' do not.

OTHER TIPS

In addition to Stephen's answer, simplify these two lines:

var midX = wordRect.x1 + (wordRect.x2 - wordRect.x1)/2.0;
var midY = wordRect.y1 + (wordRect.y2 - wordRect.y1)/2.0;

The better statements:

var midX = (wordRect.x1 + wordRect.x2)/2.0;
var midY = (wordRect.y1 + wordRect.y2)/2.0;
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top