0

I'm learning JavaScript and got to the section in my book that described creating new DOM elements like tables, rows, and columns, so I wanted to create a table similar to the periodic table of elements that I could later go back to and fiddle with as I learned new things. My problem is that, from everything that I understand so far, this code should work. Except that it doesn't display anything, and there is nothing showing up in the console in FireFox v29.0.

Ideally, I wanted to create 7 rows (periods) and 18 columns (groups), and then display the elements that were associated at those locations in the table, and then try formatting it (for example, there are empty cells inserted between elements 1 and 18, since the only elements in that period are Hydrogen (1) and Helium (2). An example of this can be seen here: http://www.webelements.com/

However, my first task (creating a basic table) just doesn't seem to work, and either I'm using the wrong terminology or no one has posted about this before (and "javascript periodic table" links mostly to just things like http://www.webelements.com/). Any ideas on what I'm missing here?

<html>
    <head>
        <title>Periodic Table</title>
    </head>
    <body>
        <script>

            var periodicTable = createTable( 7, 18);

            var period = 7;
            var group = 18;

            document.body.appendChild(periodicTable);
            function createTable( period, group ) {
                var elementId = 0;
                var table = document.createElement('table');
                for (var p = 1; p < period; ++p) {
                    var tr = table.appendChild(document.createElement('tr'));
                    for (var g = 1; g < group; ++g) {
                        var element = tr.appendChild(document.createElement('td'));
                        element.innerHTML = ++elementId;
                    }
                }
                return table;
            }
        </script>
</html>

Edit Based on the helpful comments below, I've changed the code to look like this:

        <script>

            var periodicTable = createTable( 7, 18);

            document.body.appendChild(periodicTable);
            function createTable( period, group ) {
                var elementId = 0;
                var table = document.createElement('table');
                for (var p = 1; p < period; ++p) {
                    var tr = table.appendChild(document.createElement('tr'));
                    for (var g = 1; g < group; ++g) {
                        var element = tr.appendChild(document.createElement('td'));
                        element.innerHTML = ++elementId;
                    }
                }
                return table;
            }
        </script>
SalarianEngineer
  • 624
  • 4
  • 10
  • 20
  • 4
    Why you use global variables in `createTable` instead of the passed arguments? Those globals are `undefined` at the time you use them. – Teemu May 28 '14 at 21:08
  • Ahh, I see. I've updated the code, so in this case just pass in those variables as parameters to the function. – SalarianEngineer May 28 '14 at 21:14
  • Yes, I updated it based on your comment ;-) Thank you very much. Now I just have to make it look like a real table with some CSS, and then find out how to 'skip' rows like in the real Periodic Table! Thanks for your help! – SalarianEngineer May 28 '14 at 21:15
  • Check out this topic: http://stackoverflow.com/questions/14643617/create-table-using-javascript I hope it will help ;) – Victor2748 May 28 '14 at 21:16
  • 1
    @SalarianEngineer It looks like you fixed it with your edit, but keep in mind that you can still just pass the literals to your function (7, 18) instead of assigning them to variables first. – Shawn Bush May 28 '14 at 21:18
  • 1
    Hmm... Please don't edit your post according to comments or answers, they'll become irrelevant if you do so, and future visitors might get confused. – Teemu May 28 '14 at 21:22
  • 1
    @Teemu Good idea, I'll go back and edit the changes back. Thanks for bringing that up! – SalarianEngineer May 28 '14 at 21:23
  • 1
    @Victor2748 Thanks for that link. I didn't see that when searching here, but I guess I wasn't using the right terms. One question, though: Why is there a [0] at the end of the getElementsByTagName() method? var body=document.getElementsByTagName('body')[0]; – SalarianEngineer May 28 '14 at 21:27
  • 1
    @SalarianEngineer At [MDN](https://developer.mozilla.org/en-US/docs/Web/API/Element.getElementsByTagName) you can find an explanation for that. My previous comment is perhaps slightly badly worded, I meant that editing the error off from the original code is not good. If someone asks you to add details or more information, that can be added to the post ofcourse. – Teemu May 28 '14 at 21:29
  • @Teemu I understand, thanks! And wow, I've never heard of the MDN before...time to add it to my bookmarks list! Thanks! – SalarianEngineer May 28 '14 at 22:30

1 Answers1

1

In your original code you had this:

var periodicTable = createTable( 7, 18);
var period = 7;
var group = 18;
document.body.appendChild(periodicTable);
function createTable( rows, cols ) {
                 :
    for (var p = 1; p < period; ++p) {
        var tr = table.appendChild(document.createElement('tr'));
        for (var g = 1; g < group; ++g) {...}
    }
    return table;
}

Thought vars are hoisted, i.e. variables are declared at the beginning of the scope, they are not initialized, values are assigned where ever an assignment is met. In your code period and group have value undefined until they are given values, after createTable has been executed.

You pass two arguments to createTable(), where their names are rows and cols. However, when you're needing those values, you use global variables period and group, which are undefined at the time the function is executed (they get their values after function has been finished).

To fix this, just rename the arguments rows and cols to period and group, and consider to remove the global variables (if they are not needed somewhere else ofcourse).

Teemu
  • 22,918
  • 7
  • 53
  • 106