New answers tagged

0

Array vs complex object The way you traverse your objects is convoluted. Calling Object.keys() and Object.values() all over the place like for(let i=0; i<Object.keys(markObtained).length; i++) and const data = Object.values(markObtained)[i]; can easily be avoided if you transform your input into an array. // your original students object const input = { ...


1

The code looks much simpler than I would expect for this task. I tried several examples but could neither make it fail, nor did I understand why and how this algorithm works, I was only delighted that it seems to work. Therefore I have only a few remarks. The early return for length <= 1 is not necessary. The ++rooms is missing the semicolon. Apart ...


2

If possible, always whitelist acceptable sources instead of trying to blacklist harmful ones. Also, don't use try-catch for control flow, use it for what it's designed for: error handling. All that being said, you aren't passing the promise out of the model, because you wait for the response with .then, so no you wouldn't have to make the controller ...


0

In regards to security for yourself, I would be using filter_var() to sanitize any input strings. Mainly the name and text inputs; If its a major concern or for your own sanity you could go as far as adding a preg_match($regex, $string) condition to check for characters you just don't want in those inputs. For your passwords and user security I strongly ...


2

You might notice that createChildren is almost the same as MyTree -- the only difference is that createChildren takes its argument directly instead of as a props object. So with a small change we can remove createChildren entirely: const MyTreeNestedItems = (props) => { const { data } = props; let children = null; if (data.children.length) { ...


3

In node, throwing an error without catching it will kill the process. This seems to be the primary error handling mode (and based on this it seems you then rely on pm2 to restart the process). This means that if even one client manages to trigger an error in your code, every client will be disconnected. Fixing this would require rewriting a lot of the code ...


1

Following the little big O. You ask "...if there is a better way following big o notation..." (?) Big O notation is a formalized mathematical convention used to express how a function (mathematical function) behaves as it approaches infinity. It is used in computer science to classify an algorithms complexity in regard to a definable input metric, ...


3

Access elements via ID You can get access to elements directly via their ID, if you ensure that each elements id and or name is unique withing the page and JavaScripts global scope. var x = parseFloat(document.getElementById("Lang_from").value); becomes var x = parseFloat(Lang_from.value); In the example at the bottom I have removed all the names from ...


3

Functions are your friend Even in Jython, functions are a valid construct. They allow for scope, so that you can better reason about temporary variables; give better stack traces when things go wrong; increase testability; etc. So you should move your code into some functions. Invert your logic This: if len(laty) == 7 and len(longx) == 6: should ...


4

Your compare function can be simplified a bit using Math.sign: let compare = (a, b) => { return -Math.sign(a.val - b.val); } sign will return one of -1, 0, or 1, which correspond to the sign of the number passed to it. This is basically what you were doing manually. Most languages have a sign (or signum) function for cases like this. I also used a ...


5

Let's start with how I would have done it. const data = { 0 : {"label":"New York", "val": 20}, 1 : {"label":"Rio", "val": 10}, 3 : {"label":"Tokyo", "val": 50}, 4 : {"label":"Santiago", "val": 20}, 5 : {"label":"Hong Kooong", "val": 100}, 6 : {"label":"Munich", "val": 90}, } const result = Object.values(data) .sort((a, b) => b.val - a....


0

Minor Changes Method name reduce suggests a generic API method that would take a predicate and accumulator. Instead, it's very specific in returning a custom array and mapping a string to Date. I would call it getCalendarByMinimumRecommendedTime instead. Prefer the use of const over let for immutable data: const providerCalendars and const ...


1

As suggested by others, the string approach with dot-separated properties can be ambigious. I present an alternative using lambda functions. It chains property extraction from inner objects, until a property can no longer be retrieved, in which case the default value is returned. It also performs an early exit earlier than the initial code. function extract ...


0

Your code works as intended, though your question is not clear about that. The old-skool function syntax.. function(person) { return person.age >= 16; } ..can be replaced with a lambda: p => p.age >= 16 Method rewritten: function getNamesOfLegalDrivers(people) { return people .filter(p => p.age >= 16) .map(p => p.name); } ...


2

You are not sorting in place - that is: the input array is untouched by the operation and you return a new sorted array. I would expect the input array to be sorted when the function returns. Javascript's Array.sort() behaves this way. listA.push(Infinity) listB.push(Infinity) I you encounter a problem where you are tempted to do this, you should ...


2

The problem is, that you create an array of length n for each operation (in the input array), and then sum them by calling reduce on the result and after that you finally find the maximum of that resulting array. But you only need one initial array to sum up in, and you can find the maximum in the same operation - here in a rather verbose form: function ...


2

DRY Method forEach has 4 nested if-statements, each doing something a little differently but very similar. I would take this method apart as follows. There are 2 distinct ways you are looping arg collection, so I suggest to make a method for each of them. One for looping a complex object: function forEach(collection, func, thisArg) { var i; if (...


1

Readability Review It is fine to use n to represent a count or size. It's a variable name common in mathematics, and in expansion also when programming algorithms. However, OP mixes list.length and n representing the same thing, this is bad practice as it's confusing. Furthermore: I would use const over let if a variable is immutable. Include sufficient ...


1

It works by using the output of each of the functions as the first argument to the next function. The given args are bound to the second argument and up. Just running by this description, one could easily create a piping operation with 2 helper functions: const bind = (fn, ...boundArgs) => callArg => fn(callArg, ...boundArgs) const pipe = (...fns) =&...


0

Nice little bit of fun code, and almost working. This is a long review as I got carried away. First A Bug The variable i is undeclared and thus using the global (higher level) scope. This can create very hard to find bugs in code that uses your code. Always declare all the unique variables you use in a function. You should be using strict mode via ...


0

Similar to what I mentioned in my review of your other Post: Carousel in Vanilla JavaScript, if there is only one element that has class items then it would be more appropriate to use an id attribute and query the DOM using document.getElementById(). As Ivan mentioned, when defining an arrow functions: "Parentheses are optional when there's only one ...


4

Put styles in stylesheet, and use semantic markup. It's easier to think about the problem if it is a list of input controls. You would most probably want to reset the hidden inputs, because in case a previous input value is removed, and then the form is submitted, those hidden input values would still get included. Adding a listener on a form propagates it ...


2

The most valuable property of merge sort is stability: the elements compared equal retain their relative order. The condition if(listA[i] < listB[j]){ destabilizes. If the elements happen to be equal, one from listB will be merged first. A simple fix is to rewrite the condition as if(listB[i] < listA[j]){ The Infinity trick assumes that ...


3

I don't see a way to improve the algorithm, but here are some general coding tips: A number can either be greater than 1, or less than 2. So you can use 'else' here instead. if(list.length < 2){ return list }else if(list.length > 1){ Sometimes people create extra variables to improve readability, but here 'n' is less readable than list.length. ...


0

I see some discrepancies: you say the payload is an array of bytes, yet the sample payload is a string; you say the payload is in hexadecimal format, yet the tests in code suggest otherwise. But I take it the crux of the matter is doing the same thing manually over and over again. Let's say the payload is indeed an array of hex values and you want to work ...


0

From a short review; There are 16 unused variables in your code In production code, don't call console.log, or if you have to because the code runs embedded, use a log function with a severity indication so that you can turn off or reduce logging You are missing a ton of semicolons You should jshint.com You access payload[ with magical numbers, you should ...


1

Your custom function is 36 lines of very hard to digest code. You are performing a battery of if-elseif-else conditionals on the same variable -- for this reason, it is most appropriate to employ a switch case (even though I have a strong bias against them) as a matter of best practice. If the goal is to obfuscate the output AND the code, I reckon you've ...


4

var ajax = new XMLHttpRequest(); ajax.open('POST', 'ajax.php?action=submit_form', true); You can replace XHR with fetch. While XHR is widely supported, fetch is easier to work with. It uses promises instead of callbacks, which means you can readily use async/await with it. It's inspired from jQuery's $.ajax(). So if you're familiar with that, fetch will ...


2

You could use the forEach method of URLSearchParams. Or convert it to an array using Array.from() (the spread operator would also work [...uri.searchParams] if you prefer that syntax) and use reduce. const uri = new URL('https://tempuri.org/?token=secret&test=true'); const result1 = {}; uri.searchParams.forEach((value, key) => (result1[key] = ...


4

It would be better to do your evaluation only for the particular elements that could be affected each time rather than to reevaluate all of them on any change in any input. We can rewrite this to pass along the element that triggered the event and the ID of the element that we want to update the style on. Take a look at this: function quantityCheck(...


3

Arrow elements There are two elements for the arrows, and presuming there is only one of each type (i.e. one left, one right) then it would be more appropriate to use an id attribute for distinguishing between those two. Instead of <div class="arrow-left arrow"></div> <div class="arrow-right arrow"></div> use the id attribute: &...


2

Another solution is to use the sort function of javascript. I recommend this solution for simplicity and readability var array1 = [1, 30, 4, 21, 100000]; array1.sort(() => { return b - a }); console.log(array1) // [100000, 30, 21, 4, 1] See the docs for more informations : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/...


2

I think the problem could be, that your algorithm isn't exactly a selection sort but rater a bubble sort. They are very similar, but where bubble sort swaps every two elements when the left is smaller than the right (descending order), selection sort defers swapping to after the inner loop is finished and then only swaps the last found candidate with the ...


3

Maintenance by design There are many times when I see this type of question regarding maintainability of the code. The line between code maintenance and user friendly becomes blurred. This is most evident when the coder is also the end user, the interface UI is misunderstood by familiarity of use, it is of course the IDE. This may sound somewhat pedantic, ...


2

Styling Consider using -webkit-text-stroke to make the <h2> text legible even when superimposed on a white image. Trivial errors addHoverOnEventToTItle() has improper capitalization. The image click handler calls swiperContentsInsides[index].classList.add('appear'), but the CSS has no rule for .appear. Did you mean show instead of appear? ...


2

DRY Code Your carousal is a circular construction. We can see that in your code you perform a modular incrementation/decrementation. There is a DRY way to write this code with the use of the % operator. Also, use let and const instead of var. The latter is scoped broader than you might expect. Let's get rid of this redundant code: // .. if (...


2

Feedback The code uses const for variables that don't need to be re-assigned. This is good because it prevents accidental re-assignment. It also uses === when comparing the status codes, avoiding type coercion. That is a good habit. There are at least four indentation levels because the onload callbacks are anonymous functions. Some readers of the code ...


4

Performance Strings are immutable, so using accumulator += stuffToAppend in a loop can traditionally impact performance. The problem is that we're creating a new string every iteration, leading to quadratic time complexity for an operation that should be linear. It turns out that modern browsers optimize this heavily using an internal array to represent ...


2

I personally prefer Approach 2 since it will increase readability and you can easily find out which middle-ware are applied to a route rather than to go to each excepted middle-ware and calculate. That is a lot of work for future purpose.


1

Why are you getting day in function getWeekday and never using it? Use a IIF (Immediately Invoked Function) to close over constants so they do not need to be created each time you call the function that uses them (Note optimizer will cache such constants, though not on the first pass) Don't add unneeded details to names. booking is unrelated to the functions ...


6

Here are my thoughts: Don't mutate function parameters unless there is good reason to do so. node.depth = level; The above statement basically breaks the contract of the function. The function claims to locate the deepestNode, but in fact it does findDeepestNodeAndSetDeepestPropOnAllNodes (silly, but you get the idea). This could lead to confusing, subtle ...


2

Use brackets for one-liner blocks (opinionated, but pretty strong consensus for this) Use shorthand object notation { val: val } -> { val } Use default value syntax Prefer const to let (this goes for your test too) Prefer ternary to if (not true if readability suffers) Let TreeNode take right and left as parameters module.exports = { deepestNode(node) { ...


0

Revising @Mohrn's answer: Avoid creating new objects if possible. As mentioned by @radarbob, you are excluding the closing if bracket. As such, I recommend a few changes be made: if (!error && response.statusCode == 200) { const data = JSON.parse(body); const filtered = data.articles .map( ({ title, publishedAt: date,...


1

let -> const Use the Array methods (filter, every) Use object destructuring Use object short-hand Use a method for the null check so you don't have to write it for every property if (!error && response.statusCode == 200) { const data = JSON.parse(body); const filtered = data.articles .filter(({ title, publishedAt, urlToImage, description, ...


4

Player is confusing Only 1 player is defined. This spawns the need for confusing code that looks like this.player is 0 then 1 then 2 then 3 and so on. And player-value incrementing is spread over many methods which my spidey sense says "uh-oh, player disconnects ahead!". changePlayer() { this.player = this.player === 0 ? 1 : 0; } currentPlayuer() { return ...


2

Conditional line not in brackets There is an if statement in queryAPI() without brackets. This impacts readability greatly- especially if the code to be conditionally executed gets moved to the line below. It is best to always include brackets. I haven’t tested this with your code but you could simplify the if/else to a statement that uses short-circuit ...


2

For let variable in EcmaScript6 lets you create a closure in a for loop using let. It is preferred over var, which is preferred over nothing (unless you specifically want to reuse an existing variable declared before the loop). for (index in data) { console.log(data[index], index); selectElem.options[selectElem.options.length] = new Option(data[index],...


2

Linting The code reads well. There is one convention you missed: use \s to indicate whitespace. const encode = name => { return name.replace(/ /g, "+"); }; const encode = name => { return name.replace(/\s/g, "+"); }; And the else below is redundant. if (await api.data[0]) return await api.data[0]; else return { name: [], quote: [], ...


0

So here's what I've ended up doing: INITIAL_VALUES.js function GET_FEATURE_TYPES_INITIAL_VALUES() { const FEATURE_TYPE_INITIAL_VALUES = { SINGLE_CHOICE: '', MULTIPLE_CHOICE: [], BOOLEAN: false, NUMBER: 0, STRING: '' }; return FEATURE_TYPE_INITIAL_VALUES; } function GET_FILTER_TYPES_INITIAL_VALUES() { const ...


2

Yes, it's correct to use const here. That would be considered idiomatic Javascript. If you're using const when you shouldn't, you'll notice as it will produce a TypeError. People coming from other languages might disagree as they think that const means that the variable is constant/immutable (which is not the case, it's just not re-assignable). But that is ...


Top 50 recent answers are included