0
\$\begingroup\$

I have a own made grid system and made a jQuery function for that to check if there are not too many columns in each row. If so, delete the HTML code of the row.

Normally in my grid system, there fits 12 columns in 1 row, but I can change that.

In HTML, I've written them like this:

            <div class="row">
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
            </div>

In this jQuery function, im checking the number of each [class*="size-"] and counting them up.

    function validateRowWidths() {
        $('.row').each(function (index) {
            var row = $(this),
                columns = 0;
            row.children('[class*="size-"]').each(function () {
                var eachSizeNumber = Number($(this).attr('class').match(/\d+$/)[0]);
                columns += eachSizeNumber;
            });

            row.children('[class*="offset-"]').each(function () {
                var eachSizeNumber = Number($(this).attr('class').match(/\d+$/)[0]);
                columns += eachSizeNumber;
            });

            row.children('[class*="push-"]').each(function () {
                var eachSizeNumber = Number($(this).attr('class').match(/\d+$/)[0]);
                columns += eachSizeNumber;
            });

            row.children('[class*="pull-"]').each(function () {
                var eachSizeNumber = Number($(this).attr('class').match(/\d+$/)[0]);
                columns += eachSizeNumber;
            });

            console.log(columns);

            if (columns > 12) {
                $(this).remove();
                console.log("Row " + (index + 1) + " is has to many colums in it. Remove one or more columns or make them narrower.");
            }
        });
    }

For all the grid extensions such as offset, push or pull i have to write that same each function again.

The HTML markup for the grid extensions looks like this:

            <div class="row">
                <div class="column size-1 offset-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1 push-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
                <div class="column size-1">...</div>
            </div>

Can I do this in a better way?

\$\endgroup\$
17
  • \$\begingroup\$ To be honest, this shouldn't be done with jQuery. This shouldn't be done at all in the first place. this smells of an XY problem, try defining what you are really trying to do \$\endgroup\$
    – Pinoniq
    Jun 18, 2014 at 9:11
  • \$\begingroup\$ @Pinoniq This is what i'm trying to do. But why shouldnt it be done then? \$\endgroup\$
    – Bas
    Jun 18, 2014 at 9:12
  • \$\begingroup\$ You are using client-side powered scripts to remove a div that shouldn't be there in the first place. You should have your view output correct HTML. You are writing a client-side fix for a server-side problem (the html returned could be incorrect). Hence, XY problem \$\endgroup\$
    – Pinoniq
    Jun 18, 2014 at 9:15
  • \$\begingroup\$ @Pinoniq Indeed, but if you place that 13th column, it starts to look very ugly. And the 'user' may not know how it comes right? \$\endgroup\$
    – Bas
    Jun 18, 2014 at 9:17
  • 1
    \$\begingroup\$ You are not really preventing it, you are removing the entire row if the problem occurs. That is harsh, what if someone wants it to look ugly? Simply don't add to many columns, problem solved. \$\endgroup\$
    – Pinoniq
    Jun 18, 2014 at 9:22

1 Answer 1

2
\$\begingroup\$

You could try:

  1. Using Multiple selectors
  2. Moving the calculation logic to a function so that you can re-use it elsewhere or refactor/change in a single place if the need be.

Applying this you get something like the following:

row.children('[class*="size-"],[class*="offset-"],[class*="push-"],[class*="pull-"]').each(function () {
            columns += getCurrentSize(this);
        });

 function getCurrentSize(element){
     return Number($(element).attr('class').match(/\d+$/)[0]);
 }

However, if you want each class to be evaluated separately, as mentioned in the comments, you could first try adding these elements to an array and then perform a common action on them. Something like -

var uniqueClassColumns = [];
uniqueClassColumns.push($('[class*="size-"]'));
uniqueClassColumns.push($('[class*="offset-"]'));
uniqueClassColumns.push($('[class*="push-"]'));
uniqueClassColumns.push($('[class*="pull-"]'));

uniqueClassColumns.each(function () {
                columns += getCurrentSize(this);
            });

     function getCurrentSize(element){
         return Number($(element).attr('class').match(/\d+$/)[0]);
     }
\$\endgroup\$
11
  • \$\begingroup\$ Thanks. I'll add the explanation. In the meanwhile - whats a SO-like answer? Din't get you. \$\endgroup\$
    – Vandesh
    Jun 18, 2014 at 9:00
  • 1
    \$\begingroup\$ @Vandesh here on CR we don't usually see things as problems. your answer treats the posted code as a problem and you provide a "fix". That's the SO-way of answering. here on CR it's more like we provide "guidance" on how to write cleaner, better code. That is easily understandable. We take the code that was posted, and examine it, explain what we would do different and then maybe post a different code-sample. \$\endgroup\$
    – Vogel612
    Jun 18, 2014 at 9:06
  • \$\begingroup\$ @Vandesh again welcome ;) Feel free to visit us in Code Review Chat if you have questions. Also skimming throug other answers and Code Review Meta might help you understand better what I meant ;) \$\endgroup\$
    – Vogel612
    Jun 18, 2014 at 9:08
  • \$\begingroup\$ Yup. Just stuck in the StackOverflow habit ;) Thanks again. \$\endgroup\$
    – Vandesh
    Jun 18, 2014 at 9:09
  • \$\begingroup\$ @Vandesh Thanks for this :) its much compacter, but there's one small problem. It only works for [class*="size-"]. \$\endgroup\$
    – Bas
    Jun 18, 2014 at 9:12

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.