3
\$\begingroup\$

This is all in one methods but I would like to see it in individual separate methods. The code works fine but all of the code is in the canvasApp() methods and I would like to see it all in individual methods so that I could call them as below.

Question: How to make all of the code into separate function?

canvasApp();
gameloop();

Code:

function canvasApp() {

  if (!canvasSupport()) {
             return;
        }

    function  drawScreen () {
        context.fillStyle = '#EEEEEE';
        context.fillRect(0, 0, theCanvas.width, theCanvas.height);
        //Box
        context.strokeStyle = '#000000'; 
        context.strokeRect(1,  1, theCanvas.width-2, theCanvas.height-2);
        // Create ball
        y += speed;
        context.fillStyle = "#000000";
        context.beginPath();
        context.arc(x,y,15,0,Math.PI*2,true);
        context.closePath();
        context.fill();
    }

    theCanvas = document.getElementById('canvasOne');
    context = theCanvas.getContext('2d');

    var speed = 5;
    var y = 10;
    var x = 250;

    function gameLoop() {
        window.setTimeout(gameLoop, 20);
        drawScreen();   
    }

    gameLoop();
}
\$\endgroup\$
2
\$\begingroup\$

From a Code Review perspective, what you are asking for, is worse.

Currently, you only need to call canvasApp() and that function does all the work. Inside you have drawScreen which is periodically called, starting in gameLoop.

Still,

  • I would put all the var statements on top for readability
  • I would put var in front of context and theCanvas (!)
  • I would assign your colors ('#EEEEEE') to meaningfully named variables
  • I would assign Math.PI*2 to a var doublePi = Math.PI*2 for speed
  • I would consider dropping the curly braces from the guard clause
  • //Box is not really a meaningful naming, //border and //background would make more sense`

That would give this:

function canvasApp() {

    if (!canvasSupport())
      return;

    var canvas = document.getElementById('canvasOne'),
        context = canvas.getContext('2d'), 
        borderColor = '#EEEEEE',
        backgroundColor = '#000000',
        ballColor = '#000000',
        speed = 5,
        y = 10,
        x = 250,
        doublePi = Math.PI*2,
        ballSize = 15,
        refreshInterval = 20; //In milliseconds

    function  drawScreen () {
        //Background + Border
        context.fillStyle = borderColor;
        context.fillRect(0, 0, canvas.width, canvas.height);
        //Background
        context.strokeStyle = backgroundColor; 
        context.strokeRect(1,  1, canvas.width-2, canvas.height-2);
        // Create ball
        y += speed;
        context.fillStyle = ballColor;
        context.beginPath();
        context.arc(x,y,ballSize,0,doublePi,true);
        context.closePath();
        context.fill();
    }

    function gameLoop() {
        window.setTimeout(gameLoop, refreshInterval);
        drawScreen();   
    }

    gameLoop();
}

I am still not too excited by redrawing every 20 milliseconds 2 large boxes, but I will leave the research on further optimization to you.

Update: after playing the code and reading it more closely, the code only redraws a square once, and then the border, so that should be fine.

A working jsbin version is here: the animation is fast enough, though disappointing as there is no bouncing.. http://jsbin.com/lidil/2

\$\endgroup\$
  • \$\begingroup\$ Could i just draw the boxes once which do not need to be drawn on every refresh of the screen they really are only the borders of the Canvas. \$\endgroup\$ – Doug Hauf Mar 31 '14 at 16:19
  • \$\begingroup\$ I am not sure, I think you draw a square ( well, 2 really ) to wipe out the prior ball drawing. If you create a jsfiddle then I will look into that. \$\endgroup\$ – konijn Mar 31 '14 at 17:08
  • \$\begingroup\$ The code is at home and I do have jsfiddle account I will post all of the html code to night. Thank you. \$\endgroup\$ – Doug Hauf Mar 31 '14 at 17:22
  • \$\begingroup\$ jsfiddle.net/cC3zU/1 -- here is the fiddle (I think). I am not sure how to save them and it actually was not displaying in the result window so I am not certain it is created correctly. \$\endgroup\$ – user3376708 Mar 31 '14 at 22:36
  • \$\begingroup\$ I added a fiddle where I applied my changes, click the link in my answer, if you move the mouse you will see the edit button appear in the top left corner, if you click that button you will see the source. \$\endgroup\$ – konijn Apr 2 '14 at 10:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.