5

Your code flattens the entire array of {} objects, indiscriminately flattening all properties. It does not mind, if there is only one single property, but it does not make sense semantically, especially if the array is named paintings. Rather consider the resulting array a projection/mapping to a single property painting. For a consistent array check and ...


5

why someone just write Overly Complex 2 times I don't know what format you originally received this information in, but it looks like there are 4 "tags" applied to your application: Overly Complex Solution Strange Coding Conventions Poorly Structured Hard to Understand With someone having modified the first one to include some extra information: ...


4

First off, my basic maths is a bit rusty, but is radix correct? Shouldn't Math.pow(x,1/y) be Math.pow(y,1/x)? I'm not a big fan of "extending" the native Math object. The Math object isn't a regular JavaScript object so you shouldn't assume that all implementations will allow you to add new properties to it. It would be better to create your own object and ...


4

Your reviewers are correct. You are doing way too much work here. To find the number closest to zero you only need to find the smallest absolute value in the list. You should also clarify in the interview if the list can contain non-numeric values. There's no reason to test that the values are numbers unless you need to for the purpose of the review, ...


4

I have little to add to what Pod said. There is nothing wrong with compact code. Your code could be very efficient, and there is clearly a lot of effort in it, but I agree readability could be improved. Indeed the small details count, even whitespace is important in code. The important takeaways: Good code should have a natural flow and should be visually ...


3

Normally there are a lot of categories, so I'd go through the voucher.categories array: const filterCategories = (voucher, categories) => ( Array.isArray(voucher.categories) ? voucher.categories .map(categoryId => { const categoryFound = categories.find(c => c.id === categoryId); return categoryFound ? categoryFound.name ...


3

A short review; You probably want to cache this.content.length That check for the end of array is clunky separator will change from 0 to "", an unlikely edge case, but it could happen Why are array elements in this.content ? I would have gone for this[i] instead string is not very evocative, I went with out Obligatory rewrite; Array.prototype.myJoin = ...


3

While your code is technically review-able, any review won't help you with your request, because the limited example page doesn't provide a realistic use case for your site, mostly because it doesn't have a style sheet and doesn't actually use any of the fonts, so they won't be downloaded. Why do want to have the HTML "remain legible"? (Minifying HTML ...


3

There are some issues with your class, but I don't think that they are so specific to implementing a strategy pattern. addToPage() { this.statement = document.createElement("p") this.statement.innerHTML=this.flying.fly() document.body.append(this.statement) } This assumes there is a global document variable, and that statement is part of the ...


3

There's certainly nothing wrong with using a cookie for this, but it is, alternatively, entirely possible to monitor open tabs on the client side, using localStorage. The example below listens for page load and unload events: when a page is loaded, a new entry is added to localStorage when a page is unloaded, the last entry is removed from localStorage If ...


3

Good things I like the functional approach taken with this code, and that some ecmascript-6 features like arrow functions are used. Suggestions const vs let It would be wise to default to using const for any variable that doesn't need to be re-assigned. If you later determine a value should be re-assigned then switch it to using let. This helps prevent ...


3

Good job using const for values, as well as let for re-assignable values like counter and rejected. Did you test this code? My presumption is that it didn't happen, because when I tried running it, I see the following error: error promise.then is not a function This is because the promises are spread out: function all (...promises) { Without the ...


3

The code in componentDidUpdate looks a little odd to me. if(this.state.deck.length === 0){ You are checking if there aren't any cards, then drawing some cards. This works because you are initialising the deck within getPlayerCard. What I would do is put the deck initialisation within the startNewGame method instead. getDealerCard and getPlayerCard are ...


3

I would propose something like so (given that you have assumed no dupes, etc) function migrateValues(obj, srcKey, destKey, value) { return { ...obj, [srcKey]: obj[srcKey].filter(w => w !== value), [destKey]: obj[destKey].concat(value) }; } We spread obj, and then overwrite the srcKey and destKey properties. There are ...


3

Use variable for element, toggle can take multiple classes, use minimal selector. var voteButton; if(type === 'down'){ voteButton = $('.i-down-' + commentid); voteButton.toggleClass('active-color opacity-initial'); } else { var voteButton = $('.i-up-'+commentid); voteButton.toggleClass('primary-color opacity-initial'); } if(voted === '...


3

Updating my comments on current implementation, Thanks @Mast for pointing out. You have used let for output, targetIndex and title. None of them never updated. const is better suitable here. Usage of value.title.indexOf('_') is repeated. you can use variable. You can avoid substring usage because you are already did value.title.split('_') and you can use ...


3

I've noticed you have a common bug that often occurs in Javascript code called a race condition. If uniqueIdCreatorHandler is called again before a previous call finishes there is a race condition created because state.ids is replaced at the end of the call. If both calls happen at the same time they each generate a unique copy of the original state.ids ...


3

Short review; Use a beautifier, the code is not properly indented or spaced out You use var, try to use const and let The code already assigned billAmount / noOfGuests to split, so you can compare rounding and split £ should probably be a single string constant elseIf is a terrible variable name elseIf is already multiplied by noOfGuests, why would you ...


3

Approach 2 does not work because accoring to MDN: import '/modules/my-module.js'; Is how you import a module for Import a module for its side effects only Import an entire module for side effects only, without importing anything. This runs the module's global code, but doesn't actually import any values. So, await import('/path/to/my-module.js'); won't ...


2

Strategies It would have been good to summarize the key points and strategies of the different implementations. Mostly for yourself, to clarify your thinking and solidify your understanding. Secondly for reviewers :-) Let me take a jab at that now. Solution 1 while there are lists to merge find the minimum head and merge it update the list with the ...


2

There's a slightly more concise way of filling a new array with values from a snapshot: var result = document.evaluate("//*[@*[starts-with(name(), 'temp')]]", document, null, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE); var nodes = Array.from({ length: result.snapshotLength }, (_, index) => result.snapshotItem(index)); console.log(nodes); <div ...


2

Instead of resorting the list in state just store the preferred order in state. Then in render simply sort or reverse the order as needed to present the data. Be sure to use keys that don't change when the order is changed and it'll be quite performant.


2

It may make sense to turn voucher.categories into a Set (or store them in a Set instead of an array in the first place) to speed up the contains call, however that is only sensible, if you expect it to contain a lot of items. Other than that I'd move the check for the existence of voucher.categories (and add a check if it's empty) outside the expression, in ...


2

If the list never changes once loaded, the data would only ever need to be sorted one time (which ever you prefer as the default). Once sorted, you can just reverse the array as you currently are. Additionally, if your data comes from a database of sorts, you may not even need to sort on the client at all, you could return an already sorted resultset. So ...


2

Semi-colons aren't necessarily required in JavaScript, and to be honest it's a pretty divisive topic in the JS community. Having said that, I think that it is generally considered good practice to always terminate your statements. Using things such as a beautifier or minifier will typically add semi-colons into your code where necessary/missing (depending on ...


2

let textMessage = 'Next, select the ' switch (props.sceen) { case 'dd': textMessage += 'dd' break case 'ff': textMessage += 'ff' break case 'ss': textMessage += 'ss' break default: textMessage = '' } or messageDictionary = { dd: 'Next, select the dd ', ff: 'Next, select the ff ', ss: 'Next, select the ss ' } ...


2

A small review; Once you know that one version digit is larger than the other, you can exit immediately You did not declare i with const or let I would advise the use of a beautifier for your code, it's a bit compact in some places The code does not handle well versions with different counts of digits You should always pass the base, when you call parseInt ...


2

I don't have time to give you a full review, so I'll just add some comments about checkWinner: It will read better if you remove the mutation of global state from it. Make it a pure function that returns true or false. You might consider using a 2D array as your data structure. This isn't necessary, but it maps more naturally to the visual board, and ...


2

Overall looks good. I think following improvements can be suggested. 1.Instead of this typeof ids[statementId] === "undefined" ? (ids[statementId] = 1) : ids[statementId]++; How about this for more readability ids[statementId] = (ids[statementId] || 0) + 1; 2.Instead of doing sort and immediately reverse. You can change sort function (with ...


2

Firstly, before I start, I'd like to mention that you might not need these JS animation functions. A lot of animations can be achieved through CSS transitions and keyframes, using JS to toggle classes where needed. In my opinion, JS should only step in when the animation becomes too intensive for CSS to handle. There appears to be a lot of duplication here. ...


Only top voted, non community-wiki answers of a minimum length are eligible