Read an image file into a canvas and display click coordinates

Question

I am a beginner in JavaScript, which I have started studying since Summer last year.

My code's purpose is read a file from file explorer, and put that image in a canvas. Also there is code to allow handle canvas's clicks, to allow users to display points on top of the screen.

Unit tests are at the bottom.

function getDOM() {
    var fileUpload = document.getElementById('fileUpload');
    var canvas = document.getElementById('canvas');
    var ctx = canvas.getContext("2d");
    return [fileUpload, canvas, ctx]
}

DOM = getDOM();
fileUpload = DOM[0];
canvas = DOM[1];
ctx = DOM[2];


function readImage() {
    if (isFileToRead(this)) {
        var FR = getFileReader();
        var img = getImage();
        FR.onload = fileReaderOnLoad(img);
        readBlobAsDataURLToManageItWithHtml(FR, this.files[0]);
    }
    return [this.files, FR, img, ctx];
}

function fileReaderOnLoad(img) {
    return function (e) {
        setImageSource(img, e);
        //console.log(img.src);
        img.onload = function () {
            drawImage(img, 0, 0, 512, 512);
        };
    };
}


fileUpload.onchange = readImage;


function canvasOnClick() {
    return function (e) {
        var {x, y} = getClickCoordinates(e);
        ctx.beginPath();
        ctx.fillStyle = 'black';
        drawPoint(x, y, 5, 0, Math.PI * 2, ctx);
        ctx.fill();
    };
}

canvas.onclick = canvasOnClick();

function drawPoint(x, y, width, height, diameter, ctx) {
    ctx.arc(x, y, width, height, diameter);
    return {x, y, width, height, diameter};
}
function readBlobAsDataURLToManageItWithHtml(FR, file) {
    return FR.readAsDataURL(file);
}

function isFileToRead(object) {
    return object.files && object.files[0];
}

function setImageSource(img, e) {
    img.src = e.target.result;
    return img;
}

function drawImage(img, xPos, yPos, width, height) {
    let where = img;

    ctx.drawImage(where, xPos, yPos, width, height);

    return {xPos, yPos, width, height};
}


function getClickCoordinates(e) {

    var x = e.offsetX;
    var y = e.offsetY;
    return {x, y};
}


function getFileReader() {
    const FR = new FileReader();
    return FR;
}

function getImage() {
    return new Image();
}

describe('readImage', function () {
    it('should get context ', function () {
        DOM = getDOM();
        ctx = DOM[2];
        expect(ctx).not.toBeNull();
    })
    it('should return a FileReader', function () {
        const FR = getFileReader();

        expect(FR instanceof FileReader).toBe(true);
    })
    it('should return an image', function () {
        const image = getImage();

        expect(image instanceof Image).toBe(true);
    })
    it('should return return mouse coordinates', function () {
        var e = {};
        e.offsetX = 100;
        e.offsetY = 100;
        const {x, y} = getClickCoordinates(e);

        expect(x).toBe(100);
        expect(y).toBe(100);
    })
    it('should draw point on coordinates', function () {
        img = new Image();
        xPos = yPos = 0;
        width = height = 512;

        const coordinatesOfDrawnImage = drawImage(img, xPos, yPos, width, height);

        expect(coordinatesOfDrawnImage.xPos).toBe(0);
        expect(coordinatesOfDrawnImage.yPos).toBe(0);
        expect(coordinatesOfDrawnImage.width).toBe(512);
        expect(coordinatesOfDrawnImage.height).toBe(512);

    })
    it('should prevent loading files when there are none', function () {
        let files = {};
        files.files = false;
        expect(isFileToRead(files)).toBe(false);

    })
    it('should accept loading files when there are none', function () {
        let files = [];
        files.files = [];
        files.files[0] = true;
        expect(isFileToRead(files)).toBe(true);

    })
    it('should read from fileReader to convert it into HTML URL data', function () {
        let file = new Blob;
        let FileR = new FileReader();
        readBlobAsDataURLToManageItWithHtml(FileR, file);
    })
    it('should draw a point in the given coordinates', function () {
        const x = y = height = 0;
        const width = 5;
        const diameter = Math.PI * 2;
        createCanvas();
        const newX = drawPoint(x, y, width, height, diameter, ctx);


        expect(newX.x).toBe(x);
        expect(newX.y).toBe(y);
        expect(newX.width).toBe(width);
        expect(newX.height).toBe(height);
        expect(newX.diameter).toBe(diameter);

    })
    it('should have a fileUpload input before call readImage ', function () {
        body = document.getElementsByTagName("BODY")[0];
        upload = document.createElement("input");
        upload.setAttribute("type", "file");
        upload.setAttribute("id", "fileUpload");
        body.appendChild(upload);

        readImage();
        currentFileUpload = document.getElementById('fileUpload');

        expect(currentFileUpload.toString()).toBe(upload.toString());

    })

});

The code is working fine, and passes the tests. However I do not like it at all, because of:

1. Should I use classes instead of just pure functions? If yes, why?

2. Should I isolate read functions from event handlers like the one in canvas click? If yes, why and how?

3. Should I move tests to another file, and isolate them from production code? If yes, why is it important.

4. Should be used named functions instead anonymous one? Why, and how would you write the img.onload part?

5. Should we avoid using global variables, like window or this?

6. How could we follow single responsability principle?

Answer 1

There are lots of annoying little complications in your code, for little benefit.

• What is the point of getDOM()? Why do you return three results in an array, then unpack it using three special array indexes?
• canvasOnClick() is a superfluous function call to return the real handler function.
• What is the point of getFileReader() and getImage()? A convention is that "getter" functions retrieve something that already exists; your naming violates expectations. The unit tests that verify the ability to instantiate those objects aren't worthwhile, in my opinion.
• The unit test for drawImage() doesn't really verify that the image has been drawn. Rather, it is just an exercise in echoing the input parameters back to the caller.

In summary, all of those short helper functions make it very hard to follow and understand the code. It seems like they are mostly there for the sake of unit testing, but many of the tests you have written aren't worthwhile, since they merely assert that specific lines of code work as expected, rather than observable behaviour.

I'd reorganize the code into three functions, each of which is worthy of unit-testing. Then I would connect them to specific elements of the DOM.

function drawImage(canvasCtx, imageSrc) {
    var img = new Image();
    img.onload = function() {
        canvasCtx.drawImage(img, 0, 0, 512, 512);
    };
    img.src = imageSrc;
}

function drawPoint(canvasCtx, x, y) {
    canvasCtx.beginPath();
    canvasCtx.fillStyle = 'black';
    canvasCtx.arc(x, y, 5, 0, 2 * Math.PI);
    canvasCtx.fill();
}

function loadFile(canvasCtx, files) {
    if (files && files[0]) {
        var reader = new FileReader();
        reader.onload = function() {
            drawImage(canvasCtx, reader.result);
        };
        reader.readAsDataURL(files[0]);
    }
}

//

var canvas = document.getElementById('canvas');
var ctx = canvas.getContext('2d');
var fileUpload = document.getElementById('fileUpload');

fileUpload.onchange = function fileUploadChanged() {
    loadFile(ctx, this.files);
}

canvas.onclick = function canvasClicked(event) {
    drawPoint(ctx, event.offsetX, event.offsetY);
};

canvas { border: 1px solid black; }

<form><input type="file" id="fileUpload"></form>
<canvas id="canvas" width="512" height="512"></canvas>