FrankKair / polyglot-euler

📜 Project Euler solutions in various programming languages
MIT License
74 stars 14 forks source link

Refactor: JS 001 (Functional approach) #69

Closed RamonBalthazar closed 6 years ago

RamonBalthazar commented 6 years ago

How the solution works

Enhanced the solution of problem 001 (JS) to a functional approach

Performance

Try it online results:

Real time: 0.099 s
User time: 0.086 s
Sys. time: 0.013 s
CPU share: 99.50 %
Exit code: 0

link to the test

caiangums commented 6 years ago

Well written code! Can you place a link to Try It Online on your PR description so we can see the code running? And any specific reason that now you have the semi-colon on the end of the lines? Isn't just your linter?

RamonBalthazar commented 6 years ago

Updated the PR message with the tio link :) No reason behind the semi-colons, I placed them involuntarily just for being used to. Do you think we should remove them?

caiangums commented 6 years ago

Thanks for the tio link! We can test JS on our browser with Console but for languages like Java, Clojure we need JVM, Python we need the Python Interpreter and so on. That's why I asked you to place the link 👍

Regarding the semicolons, I always heard that they are optional in several cases but I always try to put them on my JS code and let the linter of my editor do the job for me placing and removing them. My question is about the code before (that has no semicolon) and the code now that has some of them.

Thanks for your PR! I'll accept and merge your code! Feel free to keep helping us with more and more code! 😄

FrankKair commented 6 years ago

Hey, @RamonBalthazar! Thank you for contributing to polyglot-euler. We do need some more JS around here. 👍🏼

A few remarks on this pull request, though:

1 Our CONTRIBUTING.md file doesn't use ; for JS:

#!/usr/bin/env node
function func(x) {
  // Body
}

function solve() {
  // Call other functions
}

const result = solve()
console.log(result)

2 Even though the title of the pull request is Enhances solution to a functional approach, it's not really an enhancement, since it doesn't necessarily makes the code better (this is a rather broad definition), just changing the approach from structured / imperative to functional.

3 This new solution is less memory efficient, allocating an array of 1000 elements, thus making the solution O(n) in space complexity where n is the number of elements to be filtered. The older solution had no intermediate data structure, being O(1) in space.

4 The new solution is also less time efficient because we're now using fill that runs through the whole array, then we use map, that in itself is O(n), afterwards a filter (another O(n) component) and finally a reduce. I don't how fill works in JS, but using map and filter make this solution AT LEAST O(n^2) (and potentially O(n^3)!) in contrast with the latter that was linear.

For this first problem, the effect of this change is not noticeable, but as you traverse Project Euler, you'll most certainly find problems where this approach would fail harshly. So it's always nice to keep this in mind.

I'll change the name of the pull request to "Refactor: JS 001 (functional approach)" and add the JavaScript tag.