Scatterplot Graph | D3 .. Feedback Please!

Code Feedback
1

Hi all

I just finished my Scatterplot Graph project. Any feedback would be nice.

Here is a link to codepen

I would appreciate any feedback.
Thanks in advance.

2

All programmers must think alike, because I have seen this problem many times in the past - including from myself when I was making this project. There’s a problem with this layout - here’s three data points from the json file with your data…

 {
    "Time": "38:14",
    "Place": 7,
    "Seconds": 2294,
    "Name": "Miguel Indurain",
    "Year": 1995,
    "Nationality": "ESP",
    "Doping": "1994 Failed test for salbutemol, not a banned drug at that time",
    "URL": "http://www.independent.co.uk/sport/drugs-in-sport-indurain-allowed-to-use-banned-drug-1379584.html"
  },
  {
    "Time": "38:14",
    "Place": 8,
    "Seconds": 2294,
    "Name": "Alex Zülle",
    "Year": 1995,
    "Nationality": "SUI",
    "Doping": "Confessed in 1998 to taking EPO",
    "URL": "https://en.wikipedia.org/wiki/Alex_Z%C3%BClle#Festina_affair"
  },
  {
    "Time": "38:16",
    "Place": 9,
    "Seconds": 2296,
    "Name": "Bjarne Riis",
    "Year": 1995,
    "Nationality": "DEN",
    "Doping": "Alleged drug use during 1995 due to high hermatrocite levels",
    "URL": "https://en.wikipedia.org/wiki/Bjarne_Riis#Doping_allegations"
  },

If you look at your graph, at 1995 (the year of these data points), and hover over the dots - you cannot find the first one (Miguel Indurain) - it is directly behind the second one and completely hidden. So this layout doesn’t work all that great. This isn’t the only data point hidden behind a different one. It shouldn’t be too tough to rearrange the layout.

Other than that, it looks good enough - you didn’t go out of your way to make it something special, but it gets the job done. There’s a lot of extra spaces and empty lines in your code that could do without.

1 Like
3

@moT01 Thank you !!

I’ve fixed this problem by adding an Array.prototype.forEach() to iterate the data each time I hover over a circle, and find objects with the same Time and Year values.
Then adding these values to the ToolTip <div>.

If you see that I should’ve used a better way, let me know.
And please, if there is a source to it - article or piece of code-, tell me about it.

Here is the new code:

.on('mouseover', d =>{
       let similarDots = []; 
       response.forEach( obj =>{  
        if(obj.Time === d.Time && obj.Year === d.Year && obj.Place !== d.Place){
         similarDots.push(obj)
         }
        }) // end of forEach funtion

        toolTip
        .style("display", "flex")
        .style('opacity', 0.9)
        .attr('data-year', d.Year)
        .html(function(){
          if(similarDots.length === 0){
           return `${d.Name},<br> "${d.Year}"<br>${d.Doping}<br>`
          }else{
            let addedHtml = ""; 
            similarDots.forEach(obj => {
            addedHtml += `${obj.Name},<br> "${obj.Year}"<br>${obj.Doping}<br>`
            })
           return `${d.Name},<br> "${d.Year}"<br>${d.Doping}<br>--------<br>${addedHtml}`;
          }
         })
        .style('left', (d3.event.pageX + 5) +"px")
        .style("top", (d3.event.pageY - 25)  + "px")
  })

I put these spaces intentionally, just to make the code easily readable in my text editor.
Is it a bad practice? should I remove it?

Also I think the example that FCC provides in the project’s page have the same first problem, should someone notify them to fix it, or it’s okay?

4

Ahh, that makes sense now - yes, perhaps an issue could be brought up on github about the example. Here’s the old example. Your fix looks good - I can see all the data now.

I put these spaces intentionally, just to make the code easily readable in my text editor.
Is it a bad practice? should I remove it?

It’s nice to keep the formatting consistent throughout your code. Typically, there’s two spaces for indentation.

Here’s a few lines of your code…

    // Adding SVG for the Graph
    let svg = d3.select('main')
              .append('svg')
              .attr('width', svgWidth)
              .attr('height', svgHeight)
              .attr('class', 'graph-svg');
    
    
    // Creating div for the Tooltip
    let toolTip = d3.select('main')
    .append('div')
    .style('opacity', 0)
    .attr('id', 'tooltip');

I would say they should look like this…

    // Adding SVG for the Graph
    let svg = d3.select('main')
      .append('svg')
      .attr('width', svgWidth)
      .attr('height', svgHeight)
      .attr('class', 'graph-svg');
    
    // Creating div for the Tooltip
    let toolTip = d3.select('main')
      .append('div')
      .style('opacity', 0)
      .attr('id', 'tooltip');
1 Like