Hot answers tagged

10

According to www.lizard.ws the original's function cyclomatic complexity is 29 and for the second version is 22. Both numbers are usually considered high, and teams aim for much lower values (debatable what the good range is though and will see within this answer why). In order to reduce it, one way is to encapsulate the if statements, among with removing ...


8

I am not a mongoDB user but is there not some type of validation API, not sure if it can be used on schemes. If it can then maybe that is the better option for your code. The Question It have been getting complexity warnings and tried to rewrite it. Cyclomatic complexity (CC) is a count of the paths through a section of code. Note some code linters use ...


6

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 ...


6

there is no right in wrong in this situation it's a question of what's best for your particular situation if you are going to build many Rest API endpoints it's best to separate them in separate files under routes like this so your code can be more maintainable : │ ├── routes/ | | |___index.js |___auth.js |___gizmo.js ...


5

I would suggest to refactor the code to make it cleaner using a function that checks user level and limits function validateData(data) { switch(data.level) { case 0: case 1: return data.description || data.certifications.length > 0 || data.teaser || data.social.length > 0 || data.locationLat || data.locationLong || ...


5

Just addressing one of your questions. "SECURITY: Is my website secure in any way, shape, or form?" No it is not!!! Reasons Insecure transport. You should never send private data via an unsecured protocol. HTTP will let anyone see all the data communicated between you (server) and the client. YOU MUST USE HTTPS or an alternative high level encryption ...


5

Don't use callbacks, it's hard to read and maintain, the fs.readdir function will return a promise if you didn't provide a callback function. Use arrow functions when you could. Respond with an error instead of throwing (unless you have a catch wrapper middleware). Use property shorthands. You forgot req and res. Try this: const express = require('express')...


5

Review findMinimum means to me that you find the minimum value in an array of items. Since your function returns the index instead, call it indexOfMinimum. Prefer the use of const over let if you only assign a variable once: let minIndex = findMinimum(toSort); -> const minIndex = findMinimum(toSort);. Use arrow notation to write more compact functions: ...


5

I'd suggest the following changes: Use a loop instead of recursion Use for/of instead of .forEach() push a single value instead of using an array with one element in it cache the lowest value so far so you don't have to constantly refetch it on every comparison Use a temporary array for the sort so the function is non-destructive to the source array (...


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

Utilize the query object, this type of querying is more suitable there than as params. Use object destructuring. Dry your code, see how we're mapping over [countryId, stateId] to check the validity of the input. Ideally, you would have been using a module like Joi for that. Maintain a happy path and break out of the function early when erroring. Use async/...


4

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, ...


4

The main problem I see is that it's not obvious what the output is because there's too much auxiliary code that effectively obfuscates the logic. Offload some of the code into a function. Use template strings. If the data amount isn't in multi-megabyte range I would write a single string to improve readability: const dataJson = JSON.parse(data); const ...


4

Ways of improving/optimizations: What fixApostrophes and fixDashes functions actually try to do is replacing a specific punctuation chars with respective HTML entities. Instead of those numerous horrifying while loops - a more optimized, concise and extendable approach would be to: compose a predefined replacement map (where keys are search patterns and ...


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

I've taken a look at other Node Code Reviews and done some research; Use Promises instead of Callbacks. This should be an easy change, just implement a Promise in the makeAnHTTPSCallmathod and remove all callbacks. This makes error handling easier and makes the code much easier to follow: // Example usage after implementing Promises LeagueAPI....


3

After looking at your code for a while, I think I understood your requirements. They can be summarized in this table: Level Descrs Certs TSoc Other 1 0 0 no no 2 29 0 no yes 3 49 3 yes yes 4 79 5 yes yes That's the essence of your feature matrix, and that's ...


3

Your code does indeed run forever. Regrettably you don't describe the method you're using, so I will have to get that from your code. You start by getting all possible substring, of all possible sizes, out of the given input string and you store those substrings, its letters and position in an array. You do a full matrix comparison of each array entry. You ...


3

Promise chain callback hell The Promise API and now async functions are designed to reduce the number of callbacks (Colloquially known as "callback hell"). You have a total of 16 defined callbacks and by all the console logs I see you are struggling to follow the flow (callback hell) Your questions Am I using the promises correct here? Should I ...


3

Welcome to Code Review! Hopefully this experience will be useful and positive for you. Your statement about polling and then the function below basically polls my database every 1 minute looking for new rows that have a null value & performs actions on them until the lastUpdated can then be updated (once an action is performed elsewhere) I haven't ...


3

// Nothing fancy, we just wrapped the result in an array const promise1 = someAsyncOperation() const promise2 = promise1.then(result => [result]) promise2.then(result => console.log(result)) // [result] // More concisely someAsyncOperation() .then(result => [result]) .then(resultInArary => console.log(resultInArray)) // [result] Whatever ...


3

General feedback This code looks okay, though it could utilize more features of ecmascript-6, like for...of loops and default parameters (see below for more detail). Nothing jumps out as far as a way to consolidate the code in those two methods to check the collision positions but I wonder if you could consider flipping values in one of those instances in ...


3

Bearing in mind that this code was posted ~2.5 years ago, I have some feedback. Perhaps you have learned a lot since then and the information below might not be new but it might help others as well. General Feedabck Overall the code looks okay. Readability is pretty good, though the path through the helper functions is a little tricky to comprehend at ...


3

The general rule is that you should never incorporate user-controlled input into a string that will be interpreted by a computer system without escaping it first. Specifically, at a quick glance, it's obvious that… Your exec() call is vulnerable to command-line injection: exec('ping -c 2 ' + req.body.address, …) Your SQL query is vulnerable to SQL ...


3

Likely logic flaw In the first conditional: if (regex.test(pattern) && (arr.filter(s => s.includes(pattern)) != 0)) { Array.filter() "creates a new array"1 so the second conditional expression compares an array with zero. Those two should never be loosely equal so that conditional always evaluates to true. Perhaps you intended to check the ...


3

var minify = require('html-minifier').minify; const fs = require('fs'); var dir = './minifiedHTML'; Try consistently using const, let or var when declaring variables. Also, try using const first. That way, you're ensured that you are always dealing with the same thing through that variable throughout the lifetime of the app, and that any attempts to replace ...


3

I don't know about elegant, but you could load the files in parallel for better performance. To do something when all CSV files are ready you can use promises or just a simple counter. The example also shows how you can use objects as return value in your arrow functions without the extra { return {...} }. Example of loading in parallel using the counter ...


3

First the really easy advice: use strict mode https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode use const instead of let for stuff you know you're not supposed to change (symbol, player1, player2); declaring it const helps you keep some assumptions later use === instead of == generally; I think the only legitimate use for == is ...


3

think I should prefix this by saying I'm not a JS or Crypto nerd, but am interested in both! to other people seeing this question, this code/module seems to mostly reimplement the CCM demo at the bottom of the Crypto docs: https://nodejs.org/api/crypto.html#crypto_ccm_mode anyway on with a few issues I noticed: why isn't _iv just returning crypto....


3

Style Comments are just noise and thus reduce readability if they state what is obvious in the code. EG the comment //Read from each line is followed by the line lineReader.eachLine(filePath, (line)=> { Do not mix styles. You use semicolons randomly in the code. Use them or not, do not mix them Do not add empty blocks or as your comment describes "do ...


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