6
\$\begingroup\$

I've an array of objects and I want to convert it into a visual table in HTML; so I did it last night, but I was tired. I don't think it is the right way of doing it, even though it's working and gives the expected result.

let notes = [
  { note: "n1", subject: "subject1", value: 10 },
  { note: "n2", subject: "subject2", value: 15 },
  { note: "n2", subject: "subject2", value: 5 },
  { note: "n3", subject: "subject2", value: 20 },
];

function make_matrix(objs) {
  let rows = [...new Set(objs.map(({ subject }) => subject))];
  let columns = [...new Set(objs.map(({ note }) => note))];
  let array_objs = {};
  for (const obj of objs) {
    let { note, subject, value } = obj;
    if (array_objs[subject + "-" + note])
      array_objs[subject + "-" + note] = [
        ...array_objs[subject + "-" + note],
        obj,
      ];
    else array_objs[subject + "-" + note] = [obj];
  }
  for (const [k, v] of Object.entries(array_objs)) {
    total = v.map(({ value }) => +value).reduce((a, b) => a + b);
    array_objs[k] = total;
  }
  let od = {};
  for (const [k, v] of Object.entries(array_objs)) {
    const [key, value] = k.split("-");
    if (od[key]) {
      od[key] = [...od[key], { [value]: v }];
    } else {
      od[key] = [{ [value]: v }];
    }
  }
  let trs = "";
  for (const [key, value] of Object.entries(od)) {
    trs += "<tr><th>" + key + "</th>";
    for (const col of columns) {
      const entry = value
        .map((x) => Object.entries(x)[0])
        .find((x) => x[0] == col);
      if (
        value
          .map((x) => Object.entries(x)[0])
          .map((x) => x[0])
          .includes(col)
      ) {
        trs += "<td>" + entry[1] + "</td>";
      } else {
        trs += "<td>0</td>";
      }
    }
    trs += "</tr>";
  }
  let table = `<table>
                <tr><th>Subjects</th><th>${columns.join(
                  "</th><th>"
                )}</th></tr>
              ${trs}
                </table>`;
  return table;
}
document.querySelector(".content").innerHTML = make_matrix(notes);
table {
  border-collapse: collapse;
  border: 1px solid;
}
tr,
th,
td {
  border: 1px solid;
  padding: 3px 10px;
}
<div class="content"></div>

Is it the right way of doing it?

\$\endgroup\$

4 Answers 4

7
\$\begingroup\$

Use the DOM APIs

For performance avoid adding markup (HTML) to the page via JavaScript. The DOM APIs are much faster and can be abstracted to make more readable code (DOM APIs are very verbose).

Source complexity

Your solution is convoluted and hard to read. It looks like you tackled the whole problem in one big step. More than a dozen lines of code should be seen as more than one problem to solve.

To solve complex or multi step problem break them into smaller single role parts.

  • Find all rows names

  • Find all columns names

  • Find value by row column name

  • Create DOM elements

  • Append a DOM elements

  • Create a table.

Each of these sub problems can then be solved by defining a function. When you have all the functions you can solve the main problem by combining the sub problems.

Try to make the functions generic. Property names should be dynamic so that you need only minor changes when the data changes. The example show 4 tables, to change your code to provide the different tables would take a lot more work than adding 3 more calls to the main function.

Example

The rewrite breaks the problem into the pars described above. The title, and the names of the row and column properties are passed to the function.

You can see the flexibility of this approach as the table can easily be rotated and transformed by changing the argument order and properties to use for columns and rows.

const notes = [{note: "n1", subject: "subject1", value: 10 }, {note: "n2", subject: "subject2", value: 15 }, {note: "n2", subject: "subject2", value: 5 }, {note: "n3", subject: "subject2", value: 20 }];

const tag    = (tagName, props = {}) => Object.assign(document.createElement(tagName), props);
const txtTag = (tagName, str, props = {}) => tag(tagName, {textContent: str, ...props});
const append = (el, ...sibs) => sibs.reduce((p, sib) => (p.appendChild(sib), p), el);

append(document.body, 
  createTable("Subjects", "note", "subject", "value", notes),
  createTable("Notes", "subject", "note", "value", notes),
  createTable("Notes", "value", "note", "value", notes),
  createTable("Values", "subject", "value", "value", notes));

function createTable(title, colKey, rowKey, valKey, data) {
  const byKey = key => [...new Set(data.map(rec => rec[key]))];
  const byRowCol = (row, col) => data.reduce((val, rec) => 
    val + (rec[rowKey] === row && rec[colKey] === col ? rec[valKey] : 0), 0);
  const rows = byKey(rowKey), cols = byKey(colKey);
  return append(tag("table"),
    append(tag("tbody"),
      append(tag("tr"),
        txtTag("th", title),
        ...cols.map(str => txtTag("th", str))
      ),
      ...rows.map(row => 
        append(tag("tr"),
          txtTag("th", row),
          ...cols.map(col => txtTag("td", byRowCol(row, col)))
        )
      )
    )
  )
}
* {font-family: arial}
table, tr, th, td {
  border-collapse: collapse;
  border: 1px solid;
  padding: 3px 10px;
  margin: 4px;
}

\$\endgroup\$
3
\$\begingroup\$

A short review;

  • JS should be lowerCamelCase, so make_matrix -> makeMatrix
  • array_objs should probably be just be objects
  • I am not a big fan of HTML in JS, consider using a template
  • In makeMatrix you both convert the data into a new structure and build the output, I would split this across 2 functions
  • I see a tendency to name variables more after what they are than what they contain, I hope my counter-examples shows what I mean
  • I find your code very dense and hard to read
  • However I really like the trick to create subjects and notes, will steal that ;)

let notes = [
  { note: "n1", subject: "subject1", value: 10 },
  { note: "n2", subject: "subject2", value: 15 },
  { note: "n2", subject: "subject2", value: 5 },
  { note: "n3", subject: "subject2", value: 20 },
];

function restructureSubjectScores(subjectScores){
  const out = {};
  for(const score of subjectScores){
    out[score.subject] = out[score.subject] || {};
    out[score.subject][score.note] = score.value;
  }
  return out;
}

function buildTable(noteScores){
  const subjects = [...new Set(noteScores.map(({ subject }) => subject))];
  const notes = [...new Set(noteScores.map(({ note }) => note))];
  
  const tableData = restructureSubjectScores(noteScores);  
  const table = document.getElementById('tableTemplate').content.firstElementChild.cloneNode(true);
  
  const header = table.querySelector('thead tr');
  header.innerHTML += notes.map(column => `<th>${column}</th>`).join("");
  
  const tbody = table.querySelector("tbody");
  const rowTemplate = document.getElementById('rowTemplate').content.firstElementChild;

  for(const subject of subjects){
    const row = rowTemplate.cloneNode(true);
    row.querySelector("th").textContent = subject;

    for(const note of notes){
      row.innerHTML += `<td>${tableData[subject][note] || 0}</td>`;
    }
    tbody.appendChild(row);
  }
  return table;
}

document.querySelector(".content").appendChild(buildTable(notes));
table {
  border-collapse: collapse;
  border: 1px solid;
}
tr,
th,
td {
  border: 1px solid;
  padding: 3px 10px;
}
<div class="content"></div>
<template id="tableTemplate">
<table>
  <thead><tr>
    <th class="subject">Subjects</th>
  </tr></thead>
  <tbody>
  </tbody>
</table>
</template>

<template id="rowTemplate">
  <tr>
    <th class="subject"></th>
  </tr>
</template>

\$\endgroup\$
2
2
\$\begingroup\$

I think that this shows that you can vastly simplify the way that you calculate the matrix values, and to generate the table using DOM elements instead of HTML text.

let notes = [
  { note: "n1", subject: "subject1", value: 10 },
  { note: "n2", subject: "subject2", value: 15 },
  { note: "n2", subject: "subject2", value: 5 },
  { note: "n3", subject: "subject2", value: 20 },
];

function make_matrix(notes) {
    const data = notes.reduce((acc, note) => {
        const subject = acc.get(note.subject) ?? new Map()
        const value = (subject.get(note.note) || 0) + note.value
        subject.set(note.note, value)
        acc.set(note.subject, subject)
        return acc
    }, new Map)

    const rows = Array.from(new Set(notes.map(note => note.subject)))
    const cols = Array.from(new Set(notes.map(note => note.note)))

    const table = document.createElement('table')
    const thead = table.createTHead()
    const tr = thead.insertRow()
    const th = tr.appendChild(document.createElement('th'))
    th.appendChild(document.createTextNode('Subjects'))

    cols.forEach(col => {
        const th = tr.appendChild(document.createElement('th'))
        th.appendChild(document.createTextNode(col))
    })

    rows.forEach(row => {
        const tr = table.insertRow()
        const th = tr.appendChild(document.createElement('th'))
        th.appendChild(document.createTextNode(row))

        cols.forEach(col => {
            const td = tr.insertCell()
            td.appendChild(document.createTextNode(data.get(row)?.get(col) || 0))
        })
    })
    return table
}

document.querySelector(".content").appendChild(make_matrix(notes));
table {
  border-collapse: collapse;
  border: 1px solid;
}
tr,
th,
td {
  border: 1px solid;
  padding: 3px 10px;
}
<div class="content"></div>

\$\endgroup\$
0
\$\begingroup\$

Thank you for the answers. This early morning I've coded it with fewer lines; plus generated columns dynamically and also rows:

let notes = [
  { note: "n1", subject: "subject1", value: 10 },
  { note: "n2", subject: "subject2", value: 15 },
  { note: "n2", subject: "subject2", value: 5 },
  { note: "n3", subject: "subject2", value: 20 },
];

function make_matrix(notes) {
  const cols = new Set();
  notes.forEach(({ note }) => cols.add(note));

  // data object
  const data = {};
  notes.forEach(({ note, subject, value }) => {
    // if it doesn't exist, add subject property
    if (!data[subject]) {
      data[subject] = {};
      // and a property for each note with initial value 0
      [...cols].forEach((note) => (data[subject][note] = 0));
    }
    // increase value of the now definitely existing prop
    data[subject][note] += value;
  });

  // create 2-dimensional array from data object
  const arr = [["Subjects", ...cols]];
  Object.entries(data).forEach(([key, val]) => {
    arr.push([key, ...Object.values(val)]);
  });

  // output table
  const tbody = document.getElementById("tbl");
  arr.forEach((row) => {
    const tr = tbody.insertRow();
    row.forEach((val) => (tr.insertCell().innerText = val));
  });
}
make_matrix(notes);
table {
  border-collapse: collapse;
  border: 1px solid;
}
tr,
th,
td {
  border: 1px solid;
  padding: 3px 10px;
}
<table>
  <tbody id="tbl"></tbody>
</table>

\$\endgroup\$
1
  • \$\begingroup\$ Rather than post the update as an answer, it would be better to add it as a follow up question with a link back to the original question. You haven't made any comments about the original post and that makes this a bad answer. Please read How do I write a good answer? \$\endgroup\$
    – pacmaninbw
    Jun 4 at 13:15

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.