3
\$\begingroup\$

In an admin view on an intranet I maintain, there is a table of users present that allows admins to validate or delete users. Right now, the way the user is validated (or how they have their validation revoked) is by clicking a checkbox (i.e. a check in the checkbox signifies the user is validated, but no check means the user is not validated).

And right now I have an if/else statement that looks to see if the box has just been checked (or unchecked) and then sends the data to the server to achieve the desired outcome.

The code is not DRY at all, because the only difference in the code from the if/else statement is the state of the checkbox. How can I make the following code more DRY?

const chkBox = document.querySelectorAll("input[type=checkbox]");

for (box of chkBox) {
    box.addEventListener("change", updateUser, false);
}

function updateUser() {

    const messageBox = document.getElementById("message");

    if (this.checked) {

    // Getting the values to post to the server and the initialization requirements
        const postingValues = JSON.stringify({id:this.value, validated:true});
        const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

    // Making the fetch call and returning the message to the user
        fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
            response.text().then(function(value) {
                messageBox.innerHTML = value;
            });
        });
    } else {

    // Getting the values to post to the server and the initialization requirements
        const postingValues = JSON.stringify({id:this.value, validated:false});
        const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

    // Making the fetch call and returning the message to the user
        fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
            response.text().then(function(value) {
                messageBox.innerHTML = value;
            });
        });
    }
}
\$\endgroup\$
1
\$\begingroup\$

Based on your answer it seems you have figured out how to reduce the redundancy. Below are a couple other tips for improvement.

DOM reference

It is nice that the code already uses const for the variable messageBox, since that doesn’t need to be reassigned:

const messageBox = document.getElementById("message");

However I would recommend moving that outside the updateUser() function because it is looking up that DOM reference every time the function is called. If you still wanted to limit the scope to within a function then you could wrap all of that code in an IIFE, or create a function to get that DOM reference- which internally would look up the element the first time and store it for subsequent calls.

For more tips on JS that interacts with the DOM, check out this article.

Event Delegate

Also, I would recommend using event delegation (I know that article is 6 years old at the time of typing but still relevant). Your post didn’t mention how many checkboxes exist on the page or if they can be added or removed but that can be optimal for memory management.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you so much for that article, I learned a lot and I love stuff like that. And even though I figured out my original question, you broadened my view enough that I think you deserve the answer!! \$\endgroup\$ – Adam McGurk Dec 17 '17 at 20:44
3
\$\begingroup\$

So chalk this one up to me not giving it enough thought (although I've had this in the back of my mind for about two weeks and today just decided to use my Saturday to actually sit down and refactor the entire application):

The answer is that I put the state of the checkbox in a variable and I don't need to check for it at all, I can just pass it through into my postingValues object. The code now looks like this:

const chkBox = document.querySelectorAll("input[type=checkbox]");

for (box of chkBox) {
    box.addEventListener("change", updateUser, false);
}

function updateUser() {

// Getting the state of the checkbox and the DOM element to display the message
    const messageBox = document.getElementById("message");
    const validated = this.checked;

// Getting the values to post to the server and the initialization requirements
    const postingValues = JSON.stringify({id:this.value, validated:validated});
    const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

// Making the fetch call and returning the message to the user
    fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
        response.text().then(function(value) {
            messageBox.innerHTML = value;
        });
    });
}
\$\endgroup\$

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.