New answers tagged javascript
3
Before anything else, the tutorial you're referencing is this: https://www.freecodecamp.org/news/d3-and-canvas-in-3-steps-8505c8b27444/
By reading the tutorial you'll see that the elements we'll create for manipulating the canvas>...
... don’t live in the DOM but only in memory (in a ‘virtual’ DOM) and describe the life-cycle of these elements in a ...
2
Poor memory usage
Javascript has managed memory. That means it does all the hard work of allocating and releasing memory, which is great (managing memory manually is a real pain).
But this useful feature comes with a down sides...
Managed memory environments are inherently slow,
Managed memory environments encourage very poor memory usage patterns
...
1
I admit I haven't used cheerio before, but it looks like a good tool! The code looks decent. I see a couple places that can be simplified.
For example:
$('a').each(function(i, e) {
if ( $(e).attr('href').indexOf('reddit.com') === -1 &&
$(e).attr('href').indexOf('imgur.com') === -1 ) {
responseUrl = $(e).attr('href');
}
});
...
2
The function is a bit repetitive, as both cases (iterating over sub array vs individual elements) check if the element is undefined and if the return array includes that value. That could potentially be abstracted to a separate function or simplified.
It appears that the following line:
var index = props[level];
Could be moved outside of the forEach ...
0
Counter intuitive?
Infinite loops.
The function is an iterator so this may not be undesired or potentially fatal for the application. However some inputs would make one wonder about the callers intent.
numrange(0, 100, 0); // will endlessly return 0
numrange(0, 100, -1); // will return 0, -1, -2, ... Infinity
numrange(0, -100, 1); // will return 0, 1, 2, ....
2
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 ...
2
let courseFilter = this.newFilterArr.find(k => k.key === 'courseId'),
rateTypeFilter = this.newFilterArr.find(k => k.key === 'rateTypeId');
I would consider making your filter an object instead of an array. The keys are probably unique and known ahead of time, thus could be a type.
if (courseFilter && rateTypeFilter) {
return ...
1
Good things:
all lines terminated with a semi-comon
strict equality used when checking for objects
I would suggest using const by default since none of the variables/functions get reassigned. If you determine reassignment is necessary then use let. This will help avoid accidental re-assignment.
And is the variable naming oclone Hungarian notation? If so, ...
2
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 ...
4
Style
Inconsistent use of line ending semi colon
Undeclared variables parser and xmlDoc
Unused code. let t = a.children[i].textContent; t is never used the line should not be there
Poor naming
Capitalisation of variable names. Only if objects to be instanciated via new token, if accronim (eg HTML, XML) or if a constant (optional).
Examples; AuthorsInfo ...
1
In my opinion the error handling is the smallest problem with this code. The main problem is that it's very unstructured and chaotic.
Every single function is reading from (and worse, sometimes writing to) variables outside its scope. It's impossible to know which data a function is actually working with or what has changed after it has run.
A function ...
2
If you are just wanting to take the current input value and copy it to the first other input you find that begins with the same name... (Not sure this is what you are after, but...)
<!-- using onClick(),or use addListener() if you prefer -->
<input type="text" name="myName" value="abd" id=srcInput onClick="copyThis2That(this)"/>
<input type=...
2
So you don't need two loops here. You can just use one loop to
First, get the selected items
Remove the selected items from the actual array. So you can later put them in the index you want.
So to do that, first, you have to retrieve the indices where the selected obj is there. Then spice it. IN the loop
for (let index in list) {
const item = list[...
1
First some general remarks to coding conventions:
JavaScript uses camelCase identifiers, not snake_case.
Insert a space between keywords and a following opening bracket (if (...) so that they don't look like functions.
I'm not a big fan of the method name. It should be called after what is does, not what it's for. moveSelectedItemsBeforeIndex would be my ...
2
The whole thing could by simplified by adding to the HTML, for example using data attributes:
<input type="text" name="price-0" data-copy-group="price">
$(document).on('click', '.--copy', function () {
var input = $(this).closest('div').find('input');
var copyGroup = input.data('copy-group');
$('input[data-copy-group=' + copyGroup + ']')....
1
const axios = require('axios'),
fs = require('fs'),
redis = require('redis');
Personally, I discourage the use of a single var/let/const for declaring a variable. Firstly, they're not portable. Let's say you want to move fs somewhere else. Instead of copy-pasting the entire line, you end up copying that line and tacking on var/let/const anyways.
...
3
JavaScript
You don't have to list down all characters. All printable characters in the ASCII range are found in 33-126. What you could do is generate a random number within this range, and convert them into characters using String.fromCharCode().
Also, it's always a good practice to separate business logic from view logic. You will want to be able to ...
3
You should NEVER! add superfluous or unused code.
Your code should have the single if statement checking for not equal.
const href = element.getAttribute('href');
if (href.indexOf('#') !== 0) {
element.setAttribute('target', '_blank');
}
or better yet avoid the overly complex referencing via get set attribute, and not using a search to ...
2
The overall readability can be improved. As to efficiency you could remove the outer forEach and use a concat. Also the check for similarity is not necessary. Here below 3 alternative solutions, using different language primitives which make for better readability and a bit better efficiency.
Solution I: somewhat easier to read as it includes a visible for ...
1
This code looks like a good start.
Some variable could be declared with const because they aren't re-assigned - e.g. canvas, snake if the first line of newSnake() was changed to snake.length = 0;. This helps avoid accidental re-assignment in the future.
The function name pGame could be improved for readability to be more descriptive of what it does - ...
0
if you want multiple columns to hide, here is the code. i just modified Flambino's code.
<thead>
<tr>
<th class="column-a">A column</th>
<th class="column-a">Another column will hide with A</th>
<th class="column-b">Yet another column</th>
</tr>
</thead>
<button type="button" data-column="...
4
The code is mostly readable and clear:
the variable names are descriptive (for the most part - x is a little unclear)
there is good use of const and let instead of var
Some of the lines are a little lengthy - the longest line appears to be 117 characters long (excluding indentation):
newIntervals[newIntervals.length - 1][1] = Math.max(newIntervals[...
2
You state "The syntax is not the real focus..." but I can not ignore style and syntax because they are the most important parts of any good code.
Style
More use of const. For example the line var x = 100 * gridx; should be const x = 100 * gridx;
Use property shorthand when creating objects. For example defining the object literal { x: x, y: y } can be ...
3
So essentially, you want to layout an array of items in a 2D grid. This problem can be solved by using division and remainder operations to calculate the coordinates.
x is the remainder of dividing the index by maxX
y is the remainder of dividing by maxY the result of dividing the index by maxX.
A new page made is when both x and y are 0
Here's a rough ...
1
General feedback
This code looks decent, though as Andrew's answer points out many of the names are misleading. My biggest complaint is that semi-colons aren't used to terminate lines. While they are only required after a handful of statements, it could lead to errors if somehow whitespace got removed. It is a good habit to default to terminating lines with ...
1
UI
Instead of a number input, a range input could be used - with the minimum value being zero and the maximum value being half of the lesser of the height and width - in this case 88 pixels (i.e. 175px / 2). With such an input there is no need to validate user input to ensure non-numeric characters are entered.
With the current interface, the event handler ...
2
PHP
Constants
The first five variables in db.php could be declared as constants since they shouldn't be changed:
define('DB_HOST', '127.0.0.1');
define('DB_USER', '');
define('DB_PASS, '');
define('DB_NAME, 'doodle');
define('DB_CHARSET', 'utf8mb4');
And notice the prefix DB_ was added to those names to specify that those values pertain to the database. ...
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
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: ...
2
Don't repeat the queries. Store any queries that are called more than once in a variable, and utilize jQuery's chaining:
const navbarNav = $( '#navbarNav' );
const htmlBody = $( 'html, body' );
navbarNav.on( 'show.bs.collapse', function() {
htmlBody.addClass( 'noscroll' );
}).on( 'hide.bs.collapse', function() {
htmlBody.removeClass( 'noscroll' );
}...
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
In JavaScript using exceptions to communicate standard logic information is rather bad. Exceptions are bulky and slow (some browsers can not optimise code containing try blocks).
Your handling of errors will also mask real errors and thus let serious problems slip by while developing the code.
When you use exceptions always check the error in the catch to ...
6
I would limit my review to db.php file only.
First of all, I see you made use of some of my advises already, probably visited some of my articles. Sadly, not all recommendations are followed, but that's OK, we'll review them.
Connection
The connection part is simply OK, nothing to review
Structure
Regarding the structure, I would say this file is a bit ...
3
const stack = new Array()
Use the literal notation ([]) instead. Here's an answer on Stack Overflow that elaborates why you should do that instead of new Array().
const duplicate = () => {
try {
const dup = pop();
push(dup);
push(dup);
} catch(e) {
throw new Error(e);
}
};
The rule of thumb ...
3
These two functions look quite sufficient to solve the task. I will say that the functional solution will likely be slower, not only because it is functional but also because of the call to .flat() and iterating over twice as many elements.
Correct me if this is incorrect, but the call to .flat() could be removed if the call to .every() was changed to a ...
0
You seek to:
validate the url as coming from your site domain, then
replace the 3rd and 4th directories/topics with a single/new directory/topic.
This is a simple matter of preserving the substrings that you want to keep as "capture groups" and writing the new replace substring between the two capture groups.
In my demo, the new topic will be FOO. I'm ...
2
Set makes for a slow stack
Your use of a Set rather than a stack (array) is slowing everything down. The hashing function needed for Set to add delete and check items is expensive compared to pushing and popping from a stack.
JS does not like managing memory, allocations are expensive. If you use a stack you don't need to shrink it, rather just keep a ...
0
This is okay for as a start. However you are missing the two classes Bus and Ambulance.
Few comments:
(1) You are using this._variableName pattern - although not wrong this is a bit redundant way to express yourself. Historically _variableName or m_variableName comes from C++ to indicate member variables. However, this.variableName already means that this ...
2
If you wish to keep your implementation scheme, I only have one large critique:
Names are important
Your naming scheme should allow a 3rd party to understand your code from a cursory glance. You have a lot of sneaky implementations tucked into your functions that aren't expressed in your naming convention.
All of your operations are stateful, which is ...
0
Your codes looks pretty good, even though (?:I'm not really a code reviewer])*.
If you wish to use regular expressions though for validations, there are several strategies that you can follow up, given that validations with regular expressions are a bit difficult to do and it'd be good to know some details of the boundaries and limitations.
My guesswork ...
4
OO-Design
key question Identify how and where I can typically use OOP more in my job (as I can usually just use arrays and don't need objects
In your transformation towards OOP, you're still in the habit of resorting to arrays.
You let each new instance of this class be registered in some global array.
class Animal {
constructor (name, description) {
...
2
lineHandler
Prefer const over let when a variable gets assigned only once.
function lineHandlerOP(line) {
const id = Number(line.slice(0, 4));
const index = Number(line.slice(8, 10));
const maxIndex = Number(line.slice(12, 14));
const msg = line.slice(16, line.length);
return [id, index, maxIndex, msg];
}
Your implementation is very ...
4
The main suggestion I have is to take the values from sitesToBlock and put them in the @match metadata key instead of detecting when to update the document content. There is an extensive guide about match patterns here. With this approach there should not be any need to wait for the DOM to be loaded and check if the URL contains elements in sitesToBlock, ...
1
The first thing I noticed is that the format of the data is a collection of columns with rows. The variable maxNumCols seems mis-leading. A more appropriate name for that variable would be maxNumRows because it describes how many rows are needed.
In terms of reducing the transformations needed, one option to consider is finding the maximum number of rows in ...
4
I'm trying to address the same issue.
What I'm currently doing is splitting my big hooks into smaller hooks, each with a single responsability.
For example, in your example, you could split your onKeyDown, rotateRight and rotateLeft functions into a hook which takes a state with values and setters as only parameter:
const useCarRotation = state => {
...
1
This code is quite repetitive, and doesn’t adhere to the Don't Repeat Yourself principle.
One way to DRY it out is to abstract the latter promise callback to a separate function:
const addTrackAndBoundsFromKml = kmltext => {
// Create new kml overlay
const parser = new DOMParser();
kml = parser.parseFromString(kmltext,"text/xml");
...
1
I have mainly two things I'd like to look at:
I'm not a big of fan of the constant deconstructing and recreating of Sets in the loop, especially since this most likely has negative impact on the performance. Unfortunately because there is not direct way to add multiple values to a Set there is no functional "one line" solution. My function to do this would ...
2
is there a better way to achieve this functionality?
You might use a wrapping function:
let node = document.getElementById('myFavoriteNode');
node.addEventListener('click', event => Wyg.Editor.nodeClickedEvent(event));
// or, using regular function expression
node.addEventListener('click', function(event) {
return Wyg.Editor.nodeClickedEvent(event);
}...
4
Accessing statics via this
Defining the object reference outside the object just to gain access via a miss used accessor (this) is a hack and not how to use static objects to handle events.
Binding objects to a function
First what you did could have been a little less complicated
You had something like
class Editor {
static nodeClick(event){
...
2
The getter method Weapon.timePerShot will return 1 / this.fireRate. There doesn't appear to be anything to prevent fireRate from being 0. If that is the case, Weapon.timePerShot would return Infinity, which would lead to whenCanFireAgain also being set to Infinity. In some programming languages dividing by zero would lead to an exception being thrown or a ...
Top 50 recent answers are included
