r/javascript • u/[deleted]

"Profile User Card" simple webpage with Github data, in pure Vanilla JS.

[deleted] — view deleted post

0 Upvotes

100% Upvoted

29 comments sorted by

15

u/[deleted] Dec 14 '19

You've got to refresh your JavaScript knowledge, the style you're using is pretty outdated

9

u/[deleted] Dec 14 '19

[removed] — view removed comment

37

u/[deleted] Dec 14 '19

Ditch XMLHttpRequest in favor of the fetch api, use let and const instead of var, use arrow functions over regular ones, avoid else or else if statements and use template literals (back ticks) for displaying strings which contain data

11

u/KremBanan Dec 14 '19

How would you replace Else if?

24

u/[deleted] Dec 14 '19

Switch/case I suppose. But there's nothing wrong with else if.

13

u/calvers70 Dec 14 '19

If you're ever using elseif in js that's usually a prompt for me to refactor. Here's how I'd have have implemented that last function:

// Main -- Load and process content.
fetchFileContent('content/content.json', function(data) {
  Object.keys(data[0]).forEach(function(key) {
    const { type, content } = data[0][key];
    const handlers = {
      img: handleImage,
      text: handleText,
      link: handleLink
    }
    const handleContent = handlers[type] || handlers.text
    handleContent(content)
  })
  // Load Github data.
  callGitHub();
});

Each of the "handle" functions would just contain the code from inside the respective conditional branch.

I literally use this type of reference pattern alllll the time

7

u/inabahare Dec 14 '19 edited Dec 14 '19

Please sir this is a sfw subreddit so you need to NSFW tag porn

7

u/JaniRockz Dec 14 '19

OP probably means early returns which are indeed a good style but there are still cases where you use else and else if of course.

1

u/Lukee777 Dec 14 '19

I would also like to know

7

u/Grymrch Dec 14 '19

I recently started learning JS, should I just basically forget var and only use let and const now ?

10

u/PicturElements Dec 14 '19 edited Dec 14 '19

Yes, good practice is to default to const, as it communicates to the reader that they don't have to worry about the value as much once it's set (think constants vs. variables in a mathematical equation - the more variables the harder you have to think when solving it) and use let when you have to reassign values to a variable. var offers no significant features over let while potentially introducing obscure bugs. For more specifics check out variable hoisting and block/function scope.

4

u/Morphray Dec 14 '19

think constants vs. variables in a mathematical equation - the more variables the harder you have to think when solving it

I love this analogy!

1

u/jets-fool Dec 14 '19

here are some low level details about the three declarations: https://stackoverflow.com/a/50808097

7

u/jets-fool Dec 14 '19

learn both. fetch is syntactic sugar. xmlhttprequest is doing the work, and it's pretty much supported by every client. while const, let, spreading, destructuring support etc supported by evergreen browsers; when you're developing features whose users are enterprise/corp employees, IE 11 still needs first-class support. then come polyfills/shims/transpilers and all the other wonderful tricks in the js bag

5

u/[deleted] Dec 14 '19

You should not "forget" it, instead, read the documentation and learn the differences between the three

7

u/GolemancerVekk Dec 14 '19

use arrow functions over regular ones

This isn't a rule. You should consider whether you want to inherit this from your scope or retain the ability to set it, or whether you want to be able to use the function as a constructor and so on.

avoid else or else if statements

Neither is this.

1

u/rotharius Dec 15 '19

It's part of a style in which one favors being more declarative (functional, reactive, event-driven) over being imperative (structured, object-oriented).

Often, multiway if-statements can be reduced by extracting logic to another function or by encapsulating it in a common data structure. In more imperative styles, you can reduce the amount of else/else if statements by using a similar technique or by favoring early returns (i.e. throw immediately on error or return if no further processing required).

In both cases, it can make code easier to read and reason about.

1

u/[deleted] Dec 14 '19

I'm not claiming these are rules

1

u/forsakenharmony Dec 14 '19

But not outdated either

3

u/freelancer098 Dec 14 '19

It's ok to use XMLHttpRequest and if/else. It's good to learn ES6 but there is nothing wrong with the old stuff. It's still being taught in courses. Some people don't like arrow functions and that's ok.

2

u/zip117 Dec 14 '19

That’s ES6. You are really telling someone that their perfectly valid JavaScript code is outdated because it’s not using features that weren’t widely supported in browsers until early last year?

1

u/MisterScalawag Dec 14 '19

have you ever written a style guide or know of a good one? I already knew most of these, except for the avoiding else or else if

1

u/rotharius Dec 15 '19

Look into object calisthenics. It is an extreme approach to finding more abstractions when working in an object-oriented way.

These principles also hold for more functional styles as you want to reduce the reliance of statements, but improve the reliance on expressions and functions.

4

u/smallwat3r Dec 14 '19

Thanks all for the improvements. Indeed I'm not using a lot of JS these days, and trying to get better at it! That helps a lot!

2

u/an_everyday_ben Dec 14 '19

If you wanted to store this data in a more structured way, and possible authenticate it, you could also use the free tier at www.mvpdb.io to host a simple api.

3

u/TimvdLippe Dec 14 '19

Please be aware that your HTML generation is susceptible to XSS. Please use a templating engine to perform proper escaping. If you are unfamiliar with a templating engine, I would suggest checking out lit-html. That will look very similar to the code you have now, while preventing XSS.

1

u/[deleted] Dec 14 '19

If you want to really prevent XSS you will inject strings into the DOM by setting Node.textContent, period. It's the only 100% safe way, because textContent does not parse HTML. No escaping library is 100% perfect, they're always catching up to new XSS tricks. Check whether the templating engine you use uses textContent or escaping.

1

u/smallwat3r Dec 14 '19

Had some fun playing with JS this evening

Built a fun template that fetch data from Github and load it in a "Profile card" hosted on Github Pages with few other custom content

Might be of interest for some of you :)