3
\$\begingroup\$

This is my WIP attempt at an open-source web demo for exploring word vectors as part of my research. The functionality will be explained separately from the site. It is also my first time using HTML, JavaScript, and CSS in a non-trivial manner, and I am making use of modern HTML and JS features (Plotly only works with recent browsers anyway). There are several parts of my code that don't feel right to me as a programmer and in my experience lead to less clean code:

  • Heavy use of global variables: My rationale originally was that many javascript functions needed to access global variables so that they could be called by my HTML buttons. I don't think this is true but I'm not sure how I would refactor the code to avoid globals. Maybe an overarching class with attributes instead of globals? I am more familiar with OOP style but a God object may be too big?

  • CSS naming: I came up with some custom CSS naming schemes but I am sure there are better ways to name things than trying to use a unique ID for everything, in particular in javascript querySelector by CSS selectors.

  • Naming in general: I used terminology that I have seen before, but I'm not sure if it is the most accurate (ex. "process", "compute")

  • Documentation through commenting functions is still a bit lacking

  • Duplicated data throughout due to changes

  • Use of async: the only explicitly async parts are loading the main model data. The whole main function runs as async and I'm not sure this is the right approach.

I am looking to make my code cleaner and more maintainable. Hopefully answers can address my concerns above. Most computation is done beforehand so performance should not be an issue for any of the demo functions here. To run the code locally download this pre-release.

index.html

<!doctype html>
<html lang="en">
<head>
 <meta charset="utf-8">
 <title>Word2Vec Demo</title>
 <link rel="stylesheet" href="style.css" />
 <script src="https://d3js.org/d3.v7.min.js"></script>
 <script src="https://cdn.plot.ly/plotly-2.2.0.min.js"></script>
 <script src="https://cdnjs.cloudflare.com/ajax/libs/pako/2.0.3/pako.min.js"></script>
 <script src="vector.js"></script>
 <script src="word2vec.js"></script>
</head>
<body>
<h1>Word2Vec Demo</h1>
<div id="plots-wrapper">
 <div id="scatter-wrapper">
 <div id="plotly-scatter"> </div>
 <div id="scatter-buttons">
 <button id="scatter-button0" onclick="selectAxis(0)">Age</button>
 <button id="scatter-button1" onclick="selectAxis(1)">Gender</button>
 </div>
 </div>
 <div id="plotly-magnify"></div>
 <div id="plotly-vector"> </div>
</div>
<div id="plots-status-bar">
 <span id="loading-text"></span>
 <form id="word-entry">
 <input id="modify-word-input" type="text">
 <button formaction="javascript:modifyWord();" type="submit">Add/Remove Word</button>
 </form>
 <span id="modify-word-message"></span>
</div>
<details>
 <summary>Vector analogy arithmetic</summary>
 <div id="analogy-bar">
 <!-- autocomplete off to disable Firefox from saving form entries -->
 <form id="analogy-form" action="javascript:processAnalogy()" autocomplete="off">
 <!-- TODO: add proper spacing for equation -->
 <input id="analogy-word-b" type="text" placeholder="king">
 -
 <input id="analogy-word-a" type="text" placeholder="man">
 +
 <input id="analogy-word-c" type="text" placeholder="woman">
 =
 <input id="analogy-word-wstar" type="text" readonly="readonly" placeholder="queen">
 <button type="submit">Submit</button>
 </form>
 </div>
</details>
<form id="user-dimension-wrapper" action="javascript:processDimensionInput();plotScatter()">
 <div class="user-dimension-area">
 <div class="user-dimension-input">
 <label for="user-dimension-feature1-name">Dimension Name</label>
 <!-- TODO: better naming scheme in CSS -->
 <input class="user-dimension-name" id="user-dimension-feature1-name" type="text">
 </div>
 <div class="user-dimension-entry">
 <textarea class="user-dimension-feature-set" id="user-dimension-feature1-set1" rows="16"></textarea>
 <textarea class="user-dimension-feature-set" id="user-dimension-feature1-set2" rows="16"></textarea>
 </div>
 </div>
 <div class="user-dimension-area">
 <div class="user-dimension-input">
 <label for="user-dimension-feature1-name">Dimension Name</label>
 <input class="user-dimension-name" id="user-dimension-feature2-name" type="text">
 </div>
 <div class="user-dimension-entry">
 <textarea class="user-dimension-feature-set" id="user-dimension-feature2-set1" rows="16"></textarea>
 <textarea class="user-dimension-feature-set" id="user-dimension-feature2-set2" rows="16"></textarea>
 </div>
 </div>
 <button type="submit">Submit</button>
</form>
</body>
</html>

style.css

h1 {
 text-align: center;
}
#plots-wrapper {
 /* default width 100% */
 height: 70vh;
 display: grid;
 grid-template-columns: 50% 15% 35%;
}
#scatter-wrapper {
 display: flex;
 flex-direction: column;
}
#plotly-scatter {
 flex-grow: 1;
}
#plots-status-bar {
 display: grid;
 grid-template-columns: 1fr 1fr 1fr;
}
#modify-word-input {
 width: 50%
}
/* bind vector axis click */
.yaxislayer-above {
 cursor: pointer;
 pointer-events: all;
}
#user-dimension-wrapper {
 display: grid;
 grid-template-columns: 1fr 1fr;
}
.user-dimension-area {
 display: flex;
 flex-direction: column;
}
.user-dimension-entry {
 display: grid;
 grid-template-columns: 1fr 1fr;
 margin: 5px;
}
.user-dimension-feature-set {
 margin: 5px;
}

word2vec.js (main functionality)

"use strict";
const MAGNIFY_WINDOW = 5; // range for magnified view
const HEATMAP_MIN = -0.2; // min and max for heatmap colorscale
const HEATMAP_MAX = 0.2;
// to be used for naming features
let feature1Name;
let feature2Name;
// words to be used for creating dimensions
let feature1Set1, feature1Set2, feature2Set1, feature2Set2;
// Word pairs used to compute features
const FEATURE1_PAIRS =
 [
 ["man", "woman"],
 ["king", "queen"],
 ["prince", "princess"],
 ["husband", "wife"],
 ["father", "mother"],
 ["son", "daughter"],
 ["uncle", "aunt"],
 ["nephew", "niece"],
 ["boy", "girl"],
 ["male", "female"]
 ];
const FEATURE2_PAIRS =
 [
 ["man", "boy"],
 ["woman", "girl"],
 ["king", "prince"],
 ["queen", "princess"],
 ["father", "son"],
 ["mother", "daughter"],
 ["uncle", "nephew"],
 ["aunt", "niece"]
 ];
// Residual words made up from words in gender and age pairs
const RESIDUAL_WORDS = [...new Set(FEATURE1_PAIRS.flat().concat(FEATURE2_PAIRS.flat()))];
// global variables for various plotting functionality
// words plotted on scatter plot
// changes from original demo: replace "refrigerator" with "chair" and "computer"
let scatterWords = ['man', 'woman', 'boy', 'girl', 'king', 'queen', 'prince', 'princess', 'nephew', 'niece',
 'uncle', 'aunt', 'father', 'mother', 'son', 'daughter', 'husband', 'wife', 'chair', 'computer'];
// words involved in the computation of analogy in scatter plot (#12)
let analogyScatterWords = [];
// words to show in vector display
let vectorWords = ["queen", "king", "girl", "boy", "woman", "man"];
// selected word in scatterplot (empty string represents nothing selected)
let selectedWord = "";
// saved hoverX for use in magnify view
let hoverX = MAGNIFY_WINDOW;
// main word to vector Map (may include pseudo-word vectors like "man+woman")
let vecs = new Map();
// Set of actual words found in model
let vocab = new Set();
let vecsDim; // word vector dim
let nearestWords; // nearest words Map
// vector calculations and plotting, including residual (issue #3)
let feature1, feature2, residualFeature;
// read raw model text and write to vecs and vocab
function processRawVecs(text) {
 const lines = text.trim().split(/\n/);
 for (const line of lines) {
 const entries = line.trim().split(' ');
 vecsDim = entries.length - 1;
 const word = entries[0];
 const vec = new Vector(entries.slice(1).map(Number)).unit(); // normalize word vectors
 vocab.add(word);
 vecs.set(word, vec);
 }
 // sanity check for debugging input data
 RESIDUAL_WORDS.forEach(word => console.assert(vecs.has(word),word + " not in vecs"));
}
function processNearestWords(text) {
 let nearestWords = new Map();
 const lines = text.trim().split(/\n/);
 for (const line of lines) {
 const entries = line.trim().split(' ');
 const target = entries[0];
 const words = entries.slice(1);
 nearestWords.set(target, words);
 }
 return nearestWords;
}
// create feature dimension vectors
function createFeature(vecs, wordSet1, wordSet2) {
 // for each pair of words, subtract vectors
 console.assert(wordSet1.length === wordSet2.length);
 const subVecs = wordSet1.map((word1, i) => vecs.get(word1).sub(vecs.get(wordSet2[i])));
 // average subtracted vectors into one unit feature vector
 return subVecs.reduce((a,b) => a.add(b)).unit();
}
// plot each word on a 3D scatterplot projected onto gender, age, residual features
function plotScatter(newPlot=false) {
 // populate feature vectors
 feature1 = createFeature(vecs, feature1Set1, feature1Set2);
 feature2 = createFeature(vecs, feature2Set1, feature2Set2);
 const allFeatureWords = feature1Set1.concat(feature1Set2).concat(feature2Set1).concat(feature2Set2);
 const residualWords = [...new Set(allFeatureWords)];
 // residual dim calculation described in #3
 residualFeature = residualWords.map(word => {
 const wordVec = vecs.get(word);
 const wordNoFeature1 = wordVec.sub(feature1.scale(wordVec.dot(feature1)));
 const wordResidual = wordNoFeature1.sub(feature2.scale(wordNoFeature1.dot(feature2)));
 return wordResidual;
 }
 ).reduce((a,b) => a.add(b)).unit(); // average over residual words and normalize
 // add features as pseudo-words
 // TODO: not hard-code
 vecs.set("[age]", feature2);
 vecs.set("[gender]", feature1);
 // words to actually be plotted (so scatterWords is a little misleading)
 const plotWords = [...new Set(scatterWords.concat(analogyScatterWords))];
 // x, y, z are simply projections onto features
 // use 1 - residual for graphical convention (#3)
 const x = plotWords.map(word => 1 - vecs.get(word).dot(residualFeature));
 const y = plotWords.map(word => vecs.get(word).dot(feature1));
 const z = plotWords.map(word => vecs.get(word).dot(feature2));
 // color points by type with priority (#12)
 const color = plotWords.map(word =>
 (word === selectedWord) ? "#FF0000"
 : (word === analogyScatterWords[3]) ? "#FF8888"
 : (word === analogyScatterWords[4]) ? "#00FF00"
 : (analogyScatterWords.includes(word)) ? "#0000FF"
 : "#000000"
 );
 // For each point, include numbered list of nearest words in hovertext
 const hovertext = plotWords.map(target =>
 `Reference word:<br>${target}<br>` +
 "Nearest words:<br>" +
 nearestWords.get(target)
 .map((word, i) => `${i+1}. ${word}`)
 .join("<br>")
 );
 const data = [
 {
 x: x,
 y: y,
 z: z,
 mode: "markers+text",
 type: "scatter3d",
 marker: {
 size: 4,
 opacity: 0.8,
 color: color
 },
 text: plotWords,
 hoverinfo: "text",
 hovertext: hovertext
 }
 ];
 const ZOOM = 0.8;
 // save previous camera code (workaround for #9)
 let camera;
 if (newPlot) {
 camera = {eye: {x: -2.5*ZOOM, y: -0.75*ZOOM, z: 0.5*ZOOM}};
 } else { // save camera
 const plotly_scatter = document.getElementById("plotly-scatter");
 camera = plotly_scatter.layout.scene.camera;
 }
 console.log("Using camera", camera);
 const layout = {
 title: {text: "Word vector projection"},
 //uirevision: "true",
 scene: {
 xaxis: {title: "Residual", dtick: 0.1},
 yaxis: {title: "Gender", dtick: 0.1},
 zaxis: {title: "Age", dtick: 0.1},
 camera: camera
 },
 margin: {l:0, r:0, t:30, b:0}, // maximize viewing area
 font: {size: 12}
 };
 // always make new plot (#9)
 // replotting scatter3d produces ugly error (#10)
 Plotly.newPlot("plotly-scatter", data, layout);
 // bind scatter click event
 let plotly_scatter = document.getElementById("plotly-scatter");
 plotly_scatter.on("plotly_click", (data) => {
 const ptNum = data.points[0].pointNumber;
 const clickedWord = plotWords[ptNum];
 if (clickedWord === selectedWord) { // deselect
 selectedWord = "";
 console.log("Deselected", clickedWord);
 } else { // select
 selectedWord = clickedWord;
 console.log("Selected", selectedWord);
 }
 // replot with new point color
 plotScatter();
 });
}
function selectAxis(axis) {
 // TODO: cleanup
 console.log("button", axis);
 const axisNames = ["[age]", "[gender]"];
 if (selectedWord === axisNames[axis]) { // deselect word
 selectedWord = "";
 } else { // select word
 selectedWord = axisNames[axis];
 }
 // TODO: move updating button color to own function that is also called on scatter click
 for (const i of [0,1]) {
 const buttonID = "scatter-button" + i;
 document.getElementById(buttonID).style.color = (selectedWord === axisNames[i]) ? "red" : "black";
 }
 plotScatter(); // replot selected word
}
function updateHeatmapsOnWordClick() {
 // affects all heatmaps since they all have .yaxislayer-above!
 // https://stackoverflow.com/a/47400462
 console.log("Binding heatmap click event");
 d3.selectAll(".yaxislayer-above").selectAll("text")
 .on("click", (d) => {
 const idx = d.target.__data__.x;
 console.log("Clicked on", idx);
 if (selectedWord) {
 // modify vector view to show selected word and then deselect
 vectorWords[idx] = selectedWord;
 selectedWord = "";
 // replot all
 plotScatter();
 plotVector();
 }
 });
}
// plot vector and magnify views
function plotVector(newPlot=false) {
 // heatmap plots matrix of values in z
 const z = vectorWords.map(word => vecs.get(word));
 const data = [
 {
 // can't use y: vectorWords since the heatmap won't display duplicate words
 z: z,
 zmin: HEATMAP_MIN,
 zmax: HEATMAP_MAX,
 type: "heatmap",
 ygap: 5
 }
 ];
 const layout = {
 title: {text: "Vector visualization"},
 xaxis: {
 title: "Vector dimension",
 dtick: 10,
 zeroline: false,
 fixedrange: true
 },
 yaxis: {
 title: "Words",
 tickvals: d3.range(vectorWords.length),
 ticktext: vectorWords,
 fixedrange: true,
 tickangle: 60
 },
 margin: {t:30},
 //dragmode: false
 };
 if (newPlot) {
 Plotly.newPlot("plotly-vector", data, layout);
 const plotly_vector = document.getElementById("plotly-vector");
 // bind axis click to replace word in vector display after plot
 plotly_vector.on("plotly_afterplot", updateHeatmapsOnWordClick);
 plotly_vector.on("plotly_hover", data => {
 hoverX = data.points[0].x;
 console.log("Hover " + hoverX);
 plotMagnify();
 });
 plotMagnify(true);
 }
 else {
 Plotly.react("plotly-vector", data, layout);
 plotMagnify();
 }
}
function plotMagnify(newPlot=false) {
 // ensure hoverX will produce proper plot
 // bounds are inclusive
 const lo = hoverX - MAGNIFY_WINDOW;
 const hi = hoverX + MAGNIFY_WINDOW;
 if (!(0 <= lo && hi < vecsDim))
 return;
 // heatmap with subset of z
 const z = vectorWords.map(word =>
 vecs.get(word).slice(lo, hi + 1));
 const data = [
 {
 x: d3.range(lo, hi + 1),
 z: z,
 zmin: HEATMAP_MIN,
 zmax: HEATMAP_MAX,
 type: "heatmap",
 ygap: 5,
 showscale: false
 }
 ];
 const layout = {
 title: {text: "Magnified view"},
 xaxis: {
 title: "Vector dimension",
 dtick:1,
 zeroline: false,
 fixedrange: true
 },
 yaxis: {
 //title: "Words",
 tickvals: d3.range(vectorWords.length),
 ticktext: vectorWords,
 fixedrange: true,
 tickangle: 60
 },
 margin: {r:5, t:30} // get close to main vector view
 };
 if (newPlot) {
 Plotly.newPlot("plotly-magnify", data, layout);
 // bind axis click after plot, similar to vector
 const plotly_magnify = document.getElementById("plotly-magnify");
 plotly_magnify.on("plotly_afterplot", updateHeatmapsOnWordClick);
 }
 else Plotly.react("plotly-magnify", data, layout);
}
function modifyWord() {
 const word = document.getElementById("modify-word-input").value;
 let wordModified = false;
 if (scatterWords.includes(word)) { // remove word
 scatterWords = scatterWords.filter(item => item !== word);
 document.getElementById("modify-word-message").innerText = `"${word}" removed`;
 selectedWord = ""; // remove selected word
 wordModified = true;
 }
 else { // add word if in wordvecs
 if (vecs.has(word)) {
 scatterWords.push(word);
 document.getElementById("modify-word-message").innerText = `"${word}" added`;
 selectedWord = word; // make added word selected word
 wordModified = true;
 }
 else { // word not found
 document.getElementById("modify-word-message").innerText = `"${word}" not found`;
 // no replot or change to selected word
 }
 }
 if (wordModified) {
 plotScatter(); // replot to update scatter view
 document.getElementById("modify-word-input").value = ""; // clear word
 }
}
// compute 3COSADD word analogy
// also write arithmetic vectors to vector view and add nearest neighbors to result (#14)
// "Linguistic Regularities in Continuous Space Word Representations" (Mikolov 2013)
// Analogy notation for words: a:b as c:d, with d unknown
// vector y = x_b - x_a + x_c, find w* = argmax_w cossim(x_w, y)
function processAnalogy() {
 const wordA = document.getElementById("analogy-word-a").value;
 const wordB = document.getElementById("analogy-word-b").value;
 const wordC = document.getElementById("analogy-word-c").value;
 const inputWords = [wordA, wordB, wordC];
 // TODO: handle more gracefully telling user if words not available
 if (!(vecs.has(wordB) && vecs.has(wordA) && vecs.has(wordC))) {
 console.warn("bad word");
 return;
 }
 const vecA = vecs.get(wordA);
 const vecB = vecs.get(wordB);
 const vecC = vecs.get(wordC);
 // vector arithmetic, scale to unit vector
 const vecBMinusA = vecB.sub(vecA);
 const wordBMinusA = `${wordB}-${wordA}`;
 const vecY = vecBMinusA.add(vecC).unit();
 const wordY = `${wordB}-${wordA}+${wordC}`;
 // find most similar words for analogy
 let wordAnalogyPairs = [...vocab]
 .filter(word => !inputWords.includes(word)) // don't match words used in arithmetic (#12)
 .map(word => [word, vecY.dot(vecs.get(word))]);
 wordAnalogyPairs.sort((a,b) => b[1] - a[1]);
 const nearestAnalogyWords = wordAnalogyPairs.slice(0, 10).map(pair => pair[0]);
 const wordWstar = nearestAnalogyWords[0];
 // add nearest words to Y to nearest word list (#12)
 nearestWords.set(wordY, nearestAnalogyWords);
 // write out most similar word to text box
 document.getElementById("analogy-word-wstar").value = wordWstar;
 // write arithmetic vectors to vector view
 vecs.set(wordBMinusA, vecBMinusA);
 vecs.set(wordY, vecY);
 // set analogy words to display in scatter (#12) in specific order:
 analogyScatterWords = [wordB, wordA, wordC, wordY, wordWstar];
 plotScatter();
 // write arithmetic vectors to vector view (#14)
 vectorWords = [wordB, wordA, wordBMinusA, wordC, wordY, wordWstar].reverse();
 plotVector();
}
// inflate option to:"string" freezes browser, see https://github.com/nodeca/pako/issues/228
// TextDecoder may hang browser but seems much faster
function unpackVectors(vecsBuf) {
 return new Promise((resolve, reject) => {
 const vecsUint8 = pako.inflate(vecsBuf);
 const vecsText = new TextDecoder().decode(vecsUint8);
 return resolve(vecsText);
 });
}
// fill in default words used to define semantic dimensions for scatterplot
function fillDimensionDefault() {
 document.getElementById("user-dimension-feature1-set1").textContent =
 "man\nking\nprince\nhusband\nfather\nson\nuncle\nnephew\nboy\nmale";
 document.getElementById("user-dimension-feature1-set2").textContent =
 "woman\nqueen\nprincess\nwife\nmother\ndaughter\naunt\nniece\ngirl\nfemale";
 document.getElementById("user-dimension-feature2-set1").textContent =
 "man\nwoman\nking\nqueen\nfather\nmother\nuncle\naunt";
 document.getElementById("user-dimension-feature2-set2").textContent =
 "boy\ngirl\nprince\nprincess\nson\ndaughter\nnephew\nniece";
}
function processDimensionInput() {
 const feature1Set1Input = document.getElementById("user-dimension-feature1-set1").value.split('\n');
 const feature1Set2Input = document.getElementById("user-dimension-feature1-set2").value.split('\n');
 const feature2Set1Input = document.getElementById("user-dimension-feature2-set1").value.split('\n');
 const feature2Set2Input = document.getElementById("user-dimension-feature2-set2").value.split('\n');
 // TODO: user validation
 feature1Set1 = feature1Set1Input;
 feature1Set2 = feature1Set2Input;
 feature2Set1 = feature2Set1Input;
 feature2Set2 = feature2Set2Input;
 console.log(feature1Set1, feature1Set2, feature2Set1, feature2Set2);
}
// fetch wordvecs locally (no error handling) and process
async function main() {
 // fill default feature for scatterplot
 fillDimensionDefault();
 // lo-tech progress indication
 const loadingText = document.getElementById("loading-text");
 loadingText.innerText = "Downloading model...";
 // note python's http.server does not support response compression Content-Encoding
 // browsers and servers support content-encoding, but manually compress to fit on github (#1)
 const vecsResponse = await fetch("wordvecs50k.vec.gz");
 const vecsBlob = await vecsResponse.blob();
 const vecsBuf = await vecsBlob.arrayBuffer();
 // async unpack vectors
 loadingText.innerText = "Unpacking model...";
 const vecsText = await unpackVectors(vecsBuf);
 loadingText.innerText = "Processing vectors...";
 processRawVecs(vecsText);
 // fetch nearest words list
 const nearestWordsResponse = await fetch("nearest_words.txt");
 const nearestWordsText = await nearestWordsResponse.text();
 nearestWords = processNearestWords(nearestWordsText);
 loadingText.innerText = "Model processing done";
 processDimensionInput();
 // plot new plots for the first time
 plotScatter(true);
 plotVector(true);
}
// Main function runs as promise after DOM has loaded
document.addEventListener("DOMContentLoaded", () => {
 main().catch(e => console.error(e));
});
```
asked Aug 6, 2021 at 22:31
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

This a pretty big submission, so I'm going to focus on what I think is most important.

The feeling I got from skimming the code:

  • Good formatting and good use of modern javascript overall.
  • Seems to be fairly easy to read overall, though a bit hard to follow the globals.
  • Good use of domain specific names etc.
  • No overly clever bits even when there is plenty of opportunity.
  • Some functions seem to be unused, unsure about those.

I think where it falls a bit short is mainly in the architecture (you allude to this with your comment about globals), and also a bit about commenting.

Architecture

  • processRawVecs and processNearestWords use the word "process" in the name. The former doesn't return anything. It stores the result in global variables. The latter is a pure function. This is a sign to me that the word "process" doesn't have any particular meaning. It could just as well be "doStuff". I would recommend changing all pure function names to something more tangible, like getNearestWords. The goal is to create separate abstractions for separate things.
  • While we're at it, most of processRawVecs can be written as a pure function as well. The part where you add to global state can be moved out of this function. In fact, if you do this aggressively across all your functions you will get to a point where most of the logic is contained in pure functions, and where the rest of the code simply organizes the state changes. If you end up here, you most likely won't need global variables (they can go in a function or class intead), but even if you do, you will have less places that touch them (most usages of the globals will be as function parameters, not globals) The technique is called "functional core, imperative shell".
  • plotScatter is another example. There's quite a lot of logic, all of which cannot run without also displaying the plot. One consequence of this is that it's hard or impossible to test in isolation. If it's hard to test in isolation, it's often hard to think about in isolation as well.

To summarize: I would recommend that you organize your code such that

  • The code touching the dom (plotting etc) only does stuff related to that. Move the logic to pure functions.
  • Try to extract as much as possible away from the functions that touches the globals. Make pure functions where it makes sense.
  • Sandwich the pure functions (which will be most of the logic in your case) in between code that mutates the state and renders the plot.
  • Consider splitting each of view, logic, and state into three separate files to really hammer home the differences.

The result will hopefully be that you have more control over the global state, as well as more testable and understandable code. The benefits will of course depend on how large the application becomes in the end.

Comments and names

  • There's a large amount of comments. Some of them are good. For example // to be used for naming features is good because I would otherwise delete the unused variables below. Most of the comments I think are indications of unclear abstractions, missing functions, too short function names, or are just unecessary.

Snippet 1:

// create feature dimension vectors
function createFeature

It's interesting to ask why this comment needs to exist. I can see at least three possibilities:

  1. The name should have been "createFeatureDimensionVectors", because that is simply what it does (that is, a "feature" is not the correct name for this abstraction). This seems unlikely since the name "feature" is repeated all over in the rest of the code.
  2. The name "feature" is the correct name, but the abstraction is a little too fuzzy or too general, so you feel compelled to further clarify. If this is the case, the abstraction might need some more work.
  3. The comment is pointless, and can be removed

Snippet 2:

// populate feature vectors
 feature1 = createFeature(vecs, feature1Set1, feature1Set2);

Again I'm wondering what this comment is trying to achieve.

There's a bunch more of these. I would recommend reading through the comments to see if they're necessary. Maybe change a few variable names and extract a function or two as well. I think this will improve readability and reduce noise.

answered Aug 7, 2021 at 21:23
\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your comments. I'll try to separate the state changing parts because I think there is a lot of inter dependencies. Briefly: is the use of async proper? I just wrapped the entire main function as async. \$\endgroup\$ Commented Aug 7, 2021 at 21:56
  • \$\begingroup\$ Yes, looks good to me :) \$\endgroup\$ Commented Aug 8, 2021 at 8:18
  • \$\begingroup\$ What do you think about putting all my globals and functions into one Demo class? I feel like this organizes things better, even if with one class it is almost equivalent to having global variables everywhere. \$\endgroup\$ Commented Aug 9, 2021 at 20:04

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.