JavaScript DOM Implementation

Question

I'm supposed to create a website that adds multiple forms to the page, is responsive and checks if the inputs are valid (validation is not important, just needs to show some attempt at regex). Below is what I've written so far. What I'm looking for is any advice on making it more efficient and compact. Any and all help is appreciated and considered. Thanks in advance!

index.html

<!DOCTYPE html>
<html lang="en-US">
    <head>
        <meta charset="UTF-8">
        <title>index.html</title>
        <script src="script.js"></script>
        <link rel="stylesheet" type="text/css" href="style.css">
    </head>
    <body onload="execute()" id="body">
        <h3>Content Below:</h3>
        <div id="buffer">
            <div id="content">
                <!-- Content will go here -->
            </div>
        </div>
        <div id="info">
            <!-- Info will go here -->
        </div>
    </body>
</html>

script.js

function ContentDisplayer() {

    this.count = 0;

    this.show = function(id) {
        var tag = document.getElementById(id);
        tag.style.display = 'block';
    }

    this.showText = function(id, content) {
        var tag = document.getElementById(id);
        tag.innerHTML = content;
    }

    this.constructForm = function(containing_id, question) {
        //Create div containing the form
        var div_tag = document.createElement('div');
        div_tag.id = 'div_' + this.count;
        document.getElementById('body').appendChild(div_tag);

        //Create the form tag
        var form_tag = document.createElement('form');
        form_tag.id = 'form_' + this.count;
        document.getElementById(div_tag.id).appendChild(form_tag);

        //Create the fieldset tag
        var fieldset_tag = document.createElement('fieldset');
        fieldset_tag.id = 'fieldset_' + this.count;
        document.getElementById(form_tag.id).appendChild(fieldset_tag);

        //Create question label
        var label_tag = document.createElement('label');

        var label_text = document.createTextNode(question);
        label_tag.appendChild(label_text);

        label_tag.id = 'label_' + this.count;
        document.getElementById(fieldset_tag.id).appendChild(label_tag);

        //insert line break
        var break_tag = document.createElement('br');
        document.getElementById(fieldset_tag.id).appendChild(break_tag);

        //Create answer label
        var input_tag = document.createElement('input');
        input_tag.id = 'input_' + this.count;
        input_tag.type = 'text';
        document.getElementById(fieldset_tag.id).appendChild(input_tag);

        //insert line break
        var break_tag = document.createElement('br');
        document.getElementById(fieldset_tag.id).appendChild(break_tag);

        //Create button
        var button_tag = document.createElement('button');

        var button_text = document.createTextNode('Submit');
        button_tag.appendChild(button_text);

        button_tag.type = 'button';
        button_tag.id = 'button_' + this.count;

        button_tag.onclick = function() {
            var x = document.getElementById(input_tag.id);
            if(input_tag.id == 'input_0') {
                if(/^[a-zA-Z]+$/.test(x.value)) {
                    x.style.backgroundColor = "green";
                    x.style.borderColor = "green";
                }
            }
            if(input_tag.id == 'input_1') {
                if((/^[0-9]+$/.test(x.value)) && x.value > 0 && x.value <= 100) {
                    x.style.backgroundColor = "green";
                    x.style.borderColor = "green";
                }
            }
            if(input_tag.id == 'input_2') {
                if(/^\w+@[a-zA-Z_]+?\.[a-zA-Z]{2,3}$/.test(x.value)) {
                    x.style.backgroundColor = "green";
                    x.style.borderColor = "green";
                }
            }
            if(input_tag.id == 'input_3') {
                if(/\d{1,5}\s\w{1,10}\s\w{1,10}/.test(x.value)) {
                    x.style.backgroundColor = "green";
                    x.style.borderColor = "green";
                }
            }
            if(input_tag.id == 'input_4') {
                if(/^\d{3}-\d{3}-\d{4}$/.test(x.value)) {
                    x.style.backgroundColor = "green";
                    x.style.borderColor = "green";
                }
            }
        }

        document.getElementById(fieldset_tag.id).appendChild(button_tag);

        this.count += 1;
    }

}

var c;

var questions = [
    'Enter your first name',
    'Enter your age',
    'Enter your email',
    'Enter your address',
    'Enter your phone number (must use dashes): ###-###-####'
];

var question_ids = [
    'name_content',
    'age_content',
    'email_content',
    'address_content',
    'phone_content'
];

function execute() {
    c = new ContentDisplayer();
    c.show('buffer');
    c.showText('content', '<h1>Hello!</h1>');
    c.showText('info', 'If the box turns green, the information is valid!');

    //create loop to add forms to page
    for (var i = 0; i < questions.length; i++) {
        c.constructForm(question_ids[i], questions[i]);
    }
}

style.css

body {
    font-family: "Times New Roman", Times, serif;
    background-color: pink;
}

.buffer {
    display: none;
}

input[type=text] {
    border: 2px solid red;
    border-radius: 4px;
    background-color: #f44242;
    margin: 1px;
}

input[type=text]:focus {
    border: 2px solid blue;
    border-radius: 4px;
    background-color: #41dcf4;
}

button {
    background-color: green;
    border: none;
    border-radius: 6px;
    color: white;
    text-align: center;
    font-size: 16px;
}

The question title is a little misleading; my first thought was something like jsdom. — morbusg, Commented Feb 17, 2019 at 11:44

Roman · Accepted Answer · 2019-02-12 07:09:48Z

Duplicate If-Statements

You have 5 validations that look all the same. You could write a function to get ride of the duplication.

The function could look like:

function makeGreenIfValidationIsValid(tagId, regex) {
    if(input_tag.id == tagId) {
        if(regex.test(x.value)) {
            x.style.backgroundColor = "green";
            x.style.borderColor = "green";
        }
    }
}

After that, the onClick-calback would look like

button_tag.onclick = function() {
    var x = document.getElementById(input_tag.id);
    makeGreenIfValidationIsValid('input_0', /^[a-zA-Z]+$/)
    makeGreenIfValidationIsValid('input_1', /^[0-9]+$/)
    makeGreenIfValidationIsValid('input_2', /^\w+@[a-zA-Z_]+?\.[a-zA-Z]{2,3}$/)
    makeGreenIfValidationIsValid('input_3', /\d{1,5}\s\w{1,10}\s\w{1,10}/)
    makeGreenIfValidationIsValid('input_4', /^\d{3}-\d{3}-\d{4}$/)

    document.getElementById(fieldset_tag.id).appendChild(button_tag);

    this.count += 1;
}

Extract Class

The method constructForm in ContentDisplayer should be a own class. An indicator for that is that it is huge (more than 80 lines) and you have many tag-comments in it.

Comments are add all not bad but when you group your code in a method you all ready see semi-independent logic. In Robert C. Martins words: “Don’t Use a Comment When You Can Use a Function or a Variable”

For example, the class might be named "Form" and could contain several methods. Based on your comments I could look like

function Form() {

    //Create div containing the form
    this.createDivTag() {}

    //Create the form tag
    this.createFormTag() {}

    //Create the fieldset tag
    this.createFieldsetTag() {}

}

The logic in create[tag-name]Tag for creating a div, form and fieldset looks very similar. We should extract the common logic into a function.

Prototyping

Currently ContentDisplayer and Form (the class from above) don't use it.

A disadvantage is that on each creation of an object all methods like show will be recreated each time. The result is that it costs performance.

With prototyping it would look like

function ContentDisplayer() {
    this.count = 0;
}

ContentDisplayer.prototype.show = function(id) {/**/}

ContentDisplayer.prototype.showText = function(id, content) {/**/}

// ...

Compiled functions are cached. There is no performance gain by creating an external prototype. External prototypes do not allow the use of closure to create private members. Even way back when compilation was not cached external prototypes provided no benefit for single instance objects which is what is being used in this case. In modern JS external prototypes only provide a benefit when there are many instances (100+ to 1000+) with short lifetimes that are longer than a single GC cycle, with most of the small benifit in reduced GC overhead — Blindman67, Commented Feb 12, 2019 at 7:51
makeGreenIfValidationIsValid(tagId, regex) - I like the name! It's exactly what I'd come up with because I suck at being concise, so I just go with "long, boring but descriptive". Still, I'd personally inverse the order of the parameters passed in. That way you can very easily partially apply the function and derive more descriptive functions like validateNumeric = makeGreenIfValidationIsValid.bind.(this, /^[0-9]+$/) and then call validateNumeric("input_1"). — VLAZ, Commented Feb 12, 2019 at 8:07
I'd agree with extracting the form creation into its own class. But I'd go a step further and make it a Builder. So you'd probably use it like formBuilder.addDiv(this.count).addForm(this.count).addLabel().addBreak().build(). Or something similar there is probably a better way to handle this, the point is how the interface is then exposed. — VLAZ, Commented Feb 12, 2019 at 8:34
Finally, for Prototyping, I wouldn't say it's a disadvantage. The code works perfectly fine as it is. Using a prototype might help but the claim that "it costs performance" is very likely a premature optimisation. Even if it is slower, it might not matter, depending on the use case. And for a homework assignment I'd definitely not care about this aspect. I'd leave it as a preference/option but not something that's really "wrong" with the code as it is. — VLAZ, Commented Feb 12, 2019 at 8:37

morbusg · Accepted Answer · 2019-02-17 11:37:46Z

I realize the question already has an accepter answer, but I couldn’t help but notice you used the HTML5 doctype, which means you could use its new input types (such as email, tel and number) and their validation attributes (such as min, max and pattern).

That in combination with CSS’ :valid and :invalid pseudo-classes allows for real-time validation with 0 lines of javascript.

A simple quick incomplete example to illustrate above points:

<!DOCTYPE html>
<html lang="en-US">
  <head>
    <meta charset="UTF-8">

    <style>
      body { font-family: sans-serif }

      label { display: block }
      label span {
        display: inline-block;
        width: 8em;
        text-align: right;
        margin-right: .25em;
      }

      input {
        border-width: 2px;
        border-radius: 1ex;
        border-style: solid;
      }
      input:focus {
        background-color: #41dcf4;
        border-color: blue;
      }

      :valid { /* note that without a selector it applies also to whole form */
        background-color: green;
        border-color: green;
      }
      :invalid {
        background-color: #f44242;
        border-color: red;
      }
    </style>
  </head>

  <body>
  <form>
    <h1>Hello!</h1>
    <p>If the box turns green, the information is valid!</p>

    <label><span>first name</span>
      <input
        name="name"
        required
        pattern="[A-Z][a-z-]+"
      >
    </label>

    <label><span>age</span>
      <input
        type="number"
        name="age"
        required
        min="0"
        max="100"
      >
    </label>

    <label><span>email</span>
      <input
        type="email"
        name="email"
        required
        pattern="[\w-.]+@[\w-]{1,62}(\.[\w-]{1,62})*"
      >
    </label>

    <label><span>address</span>
      <input
        name="address"
        required
      >
    </label>

    <label><span>phone number</span>
      <input
        type="tel"
        name="phone"
        required
        pattern="\d{3}-\d{3}-\d{4}"
      >
    </label>

    <input type="submit">
  </form>
  </body>
</html>

Stack Exchange Network

JavaScript DOM Implementation

2 Answers 2

Duplicate If-Statements

Extract Class

Prototyping

Your Answer

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

Hot Network Questions

2 Answers 2

Duplicate If-Statements

Extract Class

Your Answer

Sign up or log in

Post as a guest

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

Related

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