1
\$\begingroup\$

I wrote a program which creates a svg-group that contains several arranged rectangles with passages inbetween to reach all of the rectangles inside of an area selected by the user. Each rectangle should get an Id.

The user can input several things like rectangle-width/height and a number which represents the ID of the first rectangle. The special thing about this ID is that the user should add 0's to the front of the ID like "01" so that all of the rectangles created by the program have the same ID-length. With "1" the program could create 9 rectangles with the same ID-length, with "01" 99, with "001" 999 and so on.

If the user does not input enough 0's in front of the ID the program should output an error. This can only be checked while I am already computing the rectangles.

My main question now is: Are the solutions a good way to handle the error? (Question A)

Furthermore, when the user inputs a falsy number, did I handle that exception in a good way? handled? (Question B)

Lastly, before the code is run I want to check if there are too many rectangles that have to be computed when e.g. the user inputs a rectangle height of "0.001". Do you think this is the best way to handle that error? (Question C)

I thought of three options:

  1. Throwing an exception if the program notices that there are not enough 0's. I think this would be the easiest to implement. My problem with that is that I read that you should only use Exceptions if there is something you could not foresee. This would not be the case here.
  2. Returning error codes out of all of the sub functions. This would be absolutely tideous and, in my opinion, make the code extremely hard to read.
  3. Setting a boolean when the function notices the error. Then checking for all of the error-booleans in the highest function and return an error-code once.

What is the best way to output these "exceptions"?

My program: I added all three types of error handlings I came up with because I'm not sure when to use which one.

function createRectangleLayout(oInputs) {
    let bNotEnoughZeros = false;

    let aExceptions = checkInputs(); //Question B
    if (aExceptions.length > 0)
        return "Falsy inputs";

    if (areTooManyEstimatedRectangles())  //Question C
        new TooManyRectanglesException();

    let layout = createNewLayout(); //Question A
    if (bNotEnoughZeros) //Is set deep inside of createNewLayout()
        return "Not enough zeros exception";

    return layout;

    function checkInputs() {
        let aExceptions = [];

        if (oInputs.rectWidth <= 0) 
            aExceptions.push("RectWidth < 0");

        if (oInputs.rectHeight <= 0) 
            aExceptions.push("RectHeight < 0");

        return aExceptions;
    }

    function areTooManyEstimatedRectangles() {
        const MAX_NR_OF_RECTS = 5000;

        let estimatedRects = estimateRects(); //returns e.g. 100
        return estimatedRects > MAX_NR_OF_RECTS;
    }

    function createLayout() {
        let aRows = calculateRows();
        let aCols = calculateCols();
        return createSVG(aRows, aCols);
    }

    function calculateRows() {
        //similar function also exists for calculateCols()
        let iRows = calculateNrOfRows();

        let aRows = [];

        for (let i = 0; i < iRows; i++) {
            if (doesNeedPassage) aRows.push("Passage");
            else aRows.push("Rectangle");
        }
    }

    function createSVG(aRows, aCols) {
        let svgGroup = createSVGGroup();
        let iRect = 0;
        let x = 0, y = 0;
        aRows.foreach((row, iRow) => {
            if (row == "Passage")
                y += oInputs.passageWidth;
            else
                aCols.foreach((col, iCol) => {
                    if (col == "Passage") 
                        x += oInputs.passageWidth;
                    else {
                        addRectToSVGGroup(svgGroup, x, y, iRect);
                        iRect++;
                    }     
                });
            x = 0;
            y += oInputs.rectHeight;
        });
    }   

    function addRectToSVGGroup(svgGroup, x, y, iRect) {
        let rect = createId(iRect);
        svgGroup.appendChild(rect);
    }

    function createId(iRect) {
        //only here the program notices if there are not enough zeros
        if (areNotEnoughZeros()) {
            bNotEnoughZeros = true;
            return "";
        } else
            return calculateId(iRect);
    }
}

This is how the function could get called:

function main() {
    try {
        let oInputs = getInputsFromForm();
        let layout = createRectangleLayout(oInputs);
        if (layout == INPUT_ERROR) 
            console.error("Input error");
        if (layout == TOO_MANY_RECTS_ERROR)
            console.error("Too many rectangles. Would take too long to compute"); 
    } catch (e) {
        console.error(e.getMessage());
    }
}
\$\endgroup\$
  • \$\begingroup\$ This is my first question here, so not quite sure how I should input the code. I left out the unnecessary functions... \$\endgroup\$ – IceRevenge Oct 7 '19 at 12:28
  • \$\begingroup\$ Your code contains syntax errors. aCols.foreach(col, iCol) => { \$\endgroup\$ – Blindman67 Oct 7 '19 at 14:19
  • \$\begingroup\$ Yes my bad. Corrected it \$\endgroup\$ – IceRevenge Oct 7 '19 at 14:25
  • \$\begingroup\$ There are some (only a quick glance so far) logic errors in the code. "passage" and "Passage" are not matching strings \$\endgroup\$ – Blindman67 Oct 7 '19 at 14:31
  • 1
    \$\begingroup\$ The title of your question defines what is to be reviewed. There is not enough code to "create a SVG-Group". Consider restating the title to better reflect what you want reviewed. BTW question (C) "How can..." is not a review question, restate as "Is my solution..." or similar and include the code that is your solution. \$\endgroup\$ – Blindman67 Oct 7 '19 at 14:56
1
\$\begingroup\$

In my opinion the error handling is the smallest problem with this code. The main problem is that it's very unstructured and chaotic.

Every single function is reading from (and worse, sometimes writing to) variables outside its scope. It's impossible to know which data a function is actually working with or what has changed after it has run.

A function should only use any data passed to it through it's parameters and never change anything "outside".

\$\endgroup\$
  • \$\begingroup\$ Ok makes sense. My problem is always that you would have to pass these arguments through 5 different functions to actually use them and sometimes some functions have 5 arguments in the end. That's why I thought using global variables might be better if you don't have too many of them... \$\endgroup\$ – IceRevenge Oct 8 '19 at 8:33
  • \$\begingroup\$ But still, what do you think is the best way of handling these errors? \$\endgroup\$ – IceRevenge Oct 8 '19 at 8:40

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.