3
\$\begingroup\$

I'm new to JavaScript and I decided to make a script that allows other developers to show SVG elements in the web browser. (It is just for learning purposes), and it is still under development.

I coded it in ES5 first and then I'm willing to upgrade it to ES6+ so I can learn and understand the differences.

I really appreciate your help to review it before I move forward and tell me what is wrong, what can I improve, if I used a design pattern in a wrong place... any help from you will be great.

So I tried to implement MVC pattern, inheritance, Factory pattern, function statement, function expression, IIFE ...

index.html:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
    <script src="rasm.js"></script>
    <script src="app.js"></script>
</body>
</html>

src lib:

var modelController = function() {'use_strict';
    var createSVGElment = function(qualifiedName) {
        return document.createElementNS('http://www.w3.org/2000/svg', qualifiedName)
    };

    var Element = function(options) {
        this.element = null;
        this.style = options.style || '';
    };
    Element.prototype.buildElement = function() {
        this.element.setAttribute('style', this.style);
    };

    var Container = function(options) {
        Element.call(this, options);
        this.height = options.height || 700;
        this.width = options.width || 1200;
        this.element = createSVGElment('svg');
    };
    Container.prototype.buildElement = function() {
        Element.prototype.buildElement.call(this);
        this.element.setAttribute('width', this.width);
        this.element.setAttribute('height', this.height);
    };

    var Rectangle = function(options) {
        Element.call(this, options);
        this.height = options.height;
        this.width = options.width;
        this.x = options.x;
        this.y = options.y;
        this.text = options.text;
        this.element = createSVGElment('rect');
    };
    Rectangle.prototype.buildElement = function() {
        Element.prototype.buildElement.call(this);
        this.element.setAttribute('x', this.x);
        this.element.setAttribute('y', this.y);
        this.element.setAttribute('width', this.width);
        this.element.setAttribute('height', this.height);
    };

    var Ellipse = function(options) {
        Element.call(this, options);
        this.x = options.cx;
        this.y = options.cy;
        this.rx = options.rx;
        this.width = options.rx * 2;
        this.ry = options.ry;
        this.height = options.ry * 2;
        this.text = options.text;
        this.element = createSVGElment('ellipse');
    };
    Ellipse.prototype.buildElement = function() {
        Element.prototype.buildElement.call(this);
        this.element.setAttribute('cx', this.x);
        this.element.setAttribute('cy', this.y);
        this.element.setAttribute('rx', this.rx);
        this.element.setAttribute('ry', this.ry);
    };

    var Link = function(options) {
        Element.call(this, options);
        this.from = options.from;
        this.to = options.to;
        this.defsElement = createSVGElment('defs');
        this.element = createSVGElment('line');
        this.camputePath = function() {
            if (this.from instanceof Ellipse) {
                this.fromX = this.from.x + this.from.rx;
                this.fromY = this.from.y;
            } else {
                this.fromX = this.from.x + this.from.width;
                this.fromY = this.from.y + (this.from.height / 2);
            }

            if (this.to instanceof Ellipse) {
                this.toX = this.to.x - this.to.rx;
                this.toY = this.to.y;
            } else {
                this.toX = this.to.x;
                this.toY = this.to.y + (this.to.height / 2);
            }
        };
    };
    Link.prototype.buildElement = function() {
        var pathEelement = createSVGElment('path');
        pathEelement.setAttribute('d', 'M0,0 L0,6 L7,3 z');
        pathEelement.setAttribute('fill', '#000');

        var markerEelement = createSVGElment('marker');
        markerEelement.setAttribute('id', 'arrow');
        markerEelement.setAttribute('markerWidth', '10');
        markerEelement.setAttribute('markerHeight', '10');
        markerEelement.setAttribute('refX', '0');
        markerEelement.setAttribute('refY', '3');
        markerEelement.setAttribute('orient', 'auto');
        markerEelement.setAttribute('markerUnits', 'strokeWidth');

        markerEelement.appendChild(pathEelement);
        this.defsElement.appendChild(markerEelement);

        this.camputePath();
        this.element.setAttribute('x1', this.fromX);
        this.element.setAttribute('y1', this.fromY);
        this.element.setAttribute('x2', this.toX - 10);
        this.element.setAttribute('y2', this.toY);
        this.element.setAttribute('stroke', '#000');
        this.element.setAttribute('stroke-width', '2');
        this.element.setAttribute('marker-end', 'url(#arrow)');
    };

    var Text = function(options) {
        Element.call(this, options);
        this.x = options.x;
        this.y = options.y;
        this.value = options.value;
        this.element = createSVGElment('text');
    };
    Text.prototype.buildElement = function() {
        Element.prototype.buildElement.call(this);
        this.element.setAttribute('x', this.x);
        this.element.setAttribute('y', this.y);
        this.element.textContent = this.value;
    };

    // Element factory
    function ElementFactory() {};
    ElementFactory.prototype.createElement = function(o) {
        switch(o.type) {
            case 'container':
                this.elementClass = Container;
                break;
            case 'rect':
                this.elementClass = Rectangle;
                break;
            case 'ellipse':
                this.elementClass = Ellipse;
                break;
            case 'link':
                this.elementClass = Link;
                break;
            case 'text':
                this.elementClass = Text;
                break;
            default:
                throw 'Warning: the type ' + o.type + ' is invalid';
        }
        return new this.elementClass(o);
    };
    var elementFactory = new ElementFactory();

    // storing register
    var register = {
        eltSVG: null,
        elts: []
    };

    return {
        registerElement: function(options) {
            var el = elementFactory.createElement(options);
            if (options.type === 'container') {
                register.eltSVG = el;
            } else {
                register.elts.push(el);
            }
            return el;
        },

        getRegister: function() {
            return register;
        },
    };
}();

var viewController = function() {'use_strict';
    return {
        displayElement: function(el) {
            var selector = el.element.tagName === 'svg' ? 'body' : 'svg';
            el.buildElement();
            if (el.defsElement) { // for line element
                document.querySelector(selector).appendChild(el.defsElement);    
            }
            document.querySelector(selector).appendChild(el.element);
        },
    };
}();

(function(global, model, view) {'use_strict';
    function processText(el, o) {
        if (['above', 'inside', 'below'].indexOf(o.text.position) < 0) {
            throw "Text position must be: above, inside or below and not " + o.text.position;
        }
        var x, y;

        if (el.element.tagName === 'rect') {
            x = el.x + (el.width * 0.1) // 20% of the width
            if (o.text.position === 'above') {
                y = el.y - 5;
            } else if (o.text.position === 'inside') {
                y = el.y + (el.height / 2);
            } else if (o.text.position === 'below') {
                y = el.y + el.height + 20;
            }
        } else { // ellipse 
            x = el.x - (el.rx * 0.2)
            if (o.text.position === 'above') {
                y = el.y - el.ry - 5;
            } else if (o.text.position === 'inside') {
                y = el.y;
            } else if (o.text.position === 'below') {
                y = el.y + el.ry + 20;
            }
        }

        return {
            x: x,
            y: y,
        }
    };

    global.$R = global.Rasm = {
        draw: function(options) {
            if (options.type !== 'container' && !model.getRegister().eltSVG) {
                throw 'You must create an svg element first';
            }
            var el = model.registerElement(options);
            view.displayElement(el);

            // process text option
            if (options.text) {
                var textCoord = processText(el, options);
                var text = model.registerElement({
                    "type": "text",
                    "x": textCoord.x,
                    "y": textCoord.y,
                    "value": options.text.value,
                });
                view.displayElement(text);
            }
            return el;
        },
    };
})(window, modelController, viewController);

An example of how we can use it: 

$R.draw({
    "type": "container",
    "height": 700,
    "width": 1100,
});

/** Left side */
var rect1 = $R.draw({
    "type": "rect",
    "x": 20,
    "y": 20,
    "width": 345,
    "height": 648,
    "style": "stroke-dasharray: 5.5; stroke: #006600; fill: #fff"
});
var rect2 = $R.draw({
    "type": "rect",
    "x": 50,
    "y": 100,
    "width": 250,
    "height": 100,
    "text": {
        "position": "inside",
        "value": "I'm a text inside a rectangle"
    },
    "style": "stroke: #006600; fill: #fff"
});
var rect3 = $R.draw({
    "type": "rect",
    "x": 50,
    "y": 300,
    "width": 250,
    "height": 100,
    "text": {
        "position": "above",
        "value": "I'm a text above a rectangle"
    },
    "style": "stroke: #006600; fill: #fff"
});
var rect4 = $R.draw({
    "type": "rect",
    "x": 50,
    "y": 500,
    "width": 250,
    "height": 100,
    "text": {
        "position": "below",
        "value": "I'm a text below a rectangle"
    },
    "style": "stroke: #006600; fill: #fff"
});

/** Right side */
var rect5 = $R.draw({
    "type": "rect",
    "x": 700,
    "y": 20,
    "width": 345,
    "height": 648,
    "style": "stroke-dasharray: 20; stroke: #006600; fill: #fff"
});
var ellipse1 = $R.draw({
    "type": "ellipse",
    "cx": 870,
    "cy": 200,
    "rx": 150,
    "ry": 70,
    "text": {
        "position": "above",
        "value": "I'm a text above an ellipse"
    },
    "style": "stroke: #129cc9; fill: #b5e5f5"
});

var ellipse2 = $R.draw({
    "type": "ellipse",
    "cx": 860,
    "cy": 500,
    "rx": 100,
    "ry": 70,
    "text": {
        "position": "inside",
        "value": "I'm a text inside an ellipse"
    },
    "style": "stroke: #129cc9; fill: #b5e5f5"
});

/** Links */
$R.draw({
    "type": "link",
    "from": rect2,
    "to": ellipse2,
});
$R.draw({
    "type": "link",
    "from": rect3,
    "to": ellipse1,
});
$R.draw({
    "type": "link",
    "from": rect4,
    "to": ellipse1,
});

the result: surjective mapping from rectangles to ellipses

I hope you concentrate only on the lib structure and code, the SVG example that I took is just to be able to practice what I have learned.

\$\endgroup\$
0

1 Answer 1

Reset to default
2
\$\begingroup\$

General feedback

For a this looks quite thorough, though I did spot a couple places where improvements can be made. See the suggestions below. It is really tempting to suggest features but I will resist the temptation because you seem to want to stick with ES5 for now.

Suggestions

Strict mode

Some functions contain:

'use_strict';

This contains an underscore but should not.1

'use strict';

And instead of adding it to each wrapping function, it could be added to the entire script.

Prototypical inheritance

Apparently Element does not have any methods that are not overridden by sub-classes. However if there were any, those would not be inherited properly. For example, say there was a method called getSides on Element:

Element.prototype.getSides = function() {
  return 3;
};

And one of the sub-classes needed to call that function

Container.prototype.buildElement = function() {
    const sides = this.getSides();

This would lead to an error:

Uncaught TypeError: this.getSides is not a function

To be thorough the sub-classes should have the prototype set to Element.prototype -

e.g.

Container.prototype = Object.create(Element.prototype);
Object.defineProperty(Container.prototype, 'constructor', { 
    value: Container, 
    enumerable: false, // so that it does not appear in 'for in' loop
    writable: true });  
Rectangle.prototype = Object.create(Element.prototype); 
Object.defineProperty(Rectangle.prototype, 'constructor', { 
    value: Rectangle, 
    enumerable: false, // so that it does not appear in 'for in' loop
    writable: true }); 
Ellipse.prototype = Object.create(Element.prototype);
Object.defineProperty(Ellipse.prototype, 'constructor', { 
    value: Ellipse, 
    enumerable: false, // so that it does not appear in 'for in' loop
    writable: true });
//... and so on...

Refer to this section of the MDN page Inheritance in JavaScript

Typo on Method name

There is a typo in the Link method camputePath - presumably it should be computePath...

Method added to each instance instead of prototype

computePath is added as a method on each instance of Link. For better performance2 it should be added to the prototype.

\$\endgroup\$
2
  • \$\begingroup\$ I really appreciate your feedback. Yes i'm not beginner to programming but i am beginner to Javascript. For now i wanted to improve this ES5 version first, and then migrate it to ES6. I'm going to alter the lib based on your suggestion thank you very much, except the Prototypical inheritance point, the Element has a buildElement method which is overridden by sub-classes, is that wrong ? \$\endgroup\$
    – Rami Belgacem
    Commented May 16, 2020 at 12:18
  • \$\begingroup\$ With the current code buildElement gets set on each class but the prototype chain isn't set properly so they aren't really overriding yet. See my updated answer with context from the linked MDN documentation. \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Commented May 19, 2020 at 22:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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