New answers tagged javascript
1
Nitpicks about naming:
pickRandomArrayNumber is a function that returns a function (and that in turn does something I actually need); it does not itself pick an "array number" (I don't actually know "array number" is either; a number from an array? if you meant index then call it index)
pickRandomItemFrom is not picking an item so don't ...
1
Naming
The name pickRandomItemFrom is too long
pick implies removal which the function does not do
from is redundant.
Thus pickRandomItemFrom can simply be randomItem with Item implying an array
Ignoring the obvious problems and removing the unnecessary currying we can rewrite your code as
const randomItem = array => array[Math.floor(Math.random() * ...
1
The code does not fully comply with what is asked in the assignment and the code readability is sometimes hard.
These critiques will be corrected throughout the code when we implement the fundamental idea to ...
Manipulate the data, not the display
Take this as a fundamental principle. Many OO design and coding buzz-word benefits naturally flow from this.
...
1
Too complex
Always strive to keep source code complexity as low as possible.
The implementation of InfiniteScroll is way too complex.
It should not be an instantiatable object as it only works if there is one instance, and will only work once.
Page counting need only one variable not 3 this.maxCount, this.numberOfPages and this.hasNextPage can be replace ...
0
I spent a ton of time on this;
The choice to replicate Date but not the other 50+ built-ins is interesting
I would write isObject as const isObject = x => (typeof x === 'object' && x !== null)
A question to ask yourself, what about functions, do you want to clone those as well?
if (Object.keys(obj).length < 1) { return obj; } is interesting, ...
1
From a short review;
Your own answer does read better
map is the right approach
I would go for filter instead of reduce (you only want 'true' values that are not keyword)
I probably would have named total -> keys
I prefer <verb><object> so combined -> addGroups
If you dont use a falsy comparison, then you dont need to check for 'keyword'
...
0
I have improved it with this:
const data = [
{
keyword: 'banana',
yellow: true,
sweet: true
},
{
keyword: 'pineapple',
yellow: true,
sweet: false
},
{
keyword: 'apple',
yellow: false,
sweet: false
},
]
const combined = [...data].map((entry) => {
const group = Object.entries(entry).reduce((total, [key,...
0
Design interfaces so that they are easy to consume, not easy to implement.
If it's easier for you to use the async variant, then there is no need to implement one with event emitter and wrap it with async function.
If it's easier to use events then implement that one.
If it's easier to use async variant but an event based implementation already exists, then ...
2
Issues
Lost resolution
You are issuing promises without a guarantee of being able to handle all or some of the resolving promises.
This will happen if promises resolve before you start the for await loop.
The offending code is in the line this.forEach(p => p.then(v => this.#resolve(v), e => this.#reject(e)));
The calls to this.#resolve and this.#...
1
Typos
caled everytime -> called every time
MAX_CARACTERS_PER_SEGMENT -> MAX_CHARACTERS_PER_SEGMENT
max_caracters -> max_characters
And my first language is not French, but shouldn't
Votre message dépasse le nombre de {{max_caracters}} caracters
be
Votre message dépasse le nombre de {{max_characters}} caractères
?
Language consistency
Your UI is in ...
0
Loved your little function and decided to give a shot at changing it. Whether you like it more or less is for you to decide, but I will try to outline principles and goals which I tried to achieve with my modification and would be happy to hear any feedback, suggestions and even criticisms are all very welcome.
So let's start with the function straight away:
...
0
Before going into more detail, I noticed the following:
The convention for naming classes is to use kebab-case. Instead of listArea I would name it list-area
Always use const instead of let unless you need to re-assign the variable. Your array is not getting reassigned
When adding the event to the window you are actually executing the function instead of ...
2
Review
Your code is a good simple solution. The style is sloppy. The complexity is a bit high and the techniques uses are negatively impacting performance.
The template literal call Array.split`+` always throws me, but I like it, your code reminds me to use it more often.
General points
Delimit all code blocks. Eg if(n==0) return ["0"] better ...
2
Some of the things I'd point out in your code for improvements:
Arrays can be a const since you will not modify the array itself but just push new elements to the array.
In your functions sortAsc, sortDesc, you are iterating through arr just to make it lowercase and iterating through the array again in sortedList.sort(). In such scenarios, think about how ...
1
Your code does not perform very well, I suppose because of the double recursion within the f-method.
It lacks readability: let's say you return to this code a year later: how long would it take to understand what you actually wanted to do?
Always use strict equality comparison (n == 0 => n === 0).
Readability can at least be enhanced by interpunction, ...
2
Some thoughts:
Writing testCode(h1.textContent = world) is a bit unconventional, and might not do what you expect: this is the same as writing the two lines
h1.textContent = world;
testCode(world);
because the assignment (the first line) evaluates to the value of the right hand side.
Instead of writing var code = codeEntered;, you can save yourself a line ...
2
Magic numbers
Avoid magic numbers. Magic numbers are just numbers in your code that represent some abstract data value. The problem is that they are spreed within the code. To make changes to these values requires searching the code and often the same values are repeated making the task even more difficult.
Defining the magic numbers as named constants makes ...
1
From a short review;
In OOP, your objects are supposed to be re-usable
I would take the x,y from a parameter
I would take the vx, vy from a parameter
I would even take the alpha from a parameter
Its nicer to call finished -> isFinished, this way the reader expects a boolean to return
In update, it makes sense to update the location with velocity, but I ...
2
Unusual undo redo
Your undo and redo operations have a strange behavior in that they are them selves undo-able. Generally undo and redo operations are themselves not undo-able.
Bug?
Unknown commands will throw the error TypeError: this.operations[operation] is not a function
SEMICOLONS!
I think I mentioned semicolons last time I reviewed your code. If you do ...
3
First off, congrats on getting your navbar working and sticking with this over several days! 🎉🥂
There are a lot of things which could be improved, but at a glance the most glaring issue is that your styles are spread all across the HTML, CSS, and JS files. This makes it very difficult to reason about them. Here's how you could simplify the situation and ...
0
A simple optimization that most programmers fail to notice:
if (...) return true else return false;
==>
return (...);
Instead of looking at every pixel, only look at each diagonal from upper left toward the south-east. (Assuming 0,0 is in the north-west.) It seems that once you hit a clear spot, the rest of the diagonal line gets an alpha of 256*0....
1
Setup:
Create an array (or database table) of all the values.
Shuffle that array.
Then, In your loop, pick the 'next' item.
And, of course, decide what to do when you run out of items. -- perhaps reshuffle and restart. Perhaps simply restart.
2
For a beginner this code looks okay. Good job using strict equality comparisons - i.e. ===. The indentation is mostly consistent, although the first indentation level is 4 spaces while all subsequent levels appear to be 2 spaces.
The biggest thing I would change is spacing. For the sake of readability, it helps to add spacing around operators - especially ...
0
Congrats on getting something like this working! I obviously don't know anything about the background of the project, so keep in mind that while my feedback may be valuable, if it's just a small side project you might not want to overengineer it.
Regardless, here are some points for you to regard:
External scripts: While embedding scripts into the document ...
2
Review
Some bad habits and inappropriate data structures used.
It is a very bad habit to leave commented code in your source.
Only use arrays when all the items represent the same thing.
Using arrays to hold structured data makes code very hard to read and sensitive to changes like item order, number of properties, etc. You use arrays for the data passed ...
1
Let's take a look at that first line of the PageDirection function:
var self = this;
This is a common practice to keep a reference to the this object within the closure. A different way to do this would be to bind the context of the closure using Function.bind()
this.disabled = function (currentPage, lastPage) {
return this.value() > lastPage || ...
1
First of all, great job on the game! Even though it's a simple concept it feels really well realized by you.
Let's go over your questions one by one:
1. UI & UX
Well done on this one! Simple, clean design, sufficient spacing on elements, consistent styling, nice visual appeal. Obviously not optimized for mobile devices, but that's a plus for the future. ...
1
If you need to account for mixed response types (in your case "unknown errors" or JSON-formatted errors), then you could consider checking the Content-Type of the response. See this StackOverflow answer for reference. This, of course, trusts in the server echoing the correct content type header for a response.
As you mentioned, fetch does not ...
2
SELECT COUNT(DISTINCT comment_post_ID)
FROM ".$wpdb->prefix."comments USE INDEX (left_frame_index)
WHERE comment_approved = 1
AND comment_type = 'comment'
LIMIT $count_limit
Needs a better index; see the below.
Do NOT compare a VARCHAR (such as comment_approved) with a numeric literal (such as 1), the index cannot be used....
1
Class Structure
Objectives and checkpoints [...] are part of the calendar object.
The given description would result in the following (simplified) UML-Diagram, where the Calendar is an aggregate of Objective and Checkpoint:
But the code would give us the following (simplified) UML-Diagram, where the Calendar knows Objective and Checkpoint and vise versa:
...
1
I notice that you have many member functions entitled getX or setX. It would be much more idiomatic in JavaScript to use getters and setters for this purpose.
e.g. instead of getObjective
get objective() {
for (const objective of calendar.objectives) {
if (objective.id === this.objectiveid) {
return objective;
}
}
}
Edit: ...
1
One problem with Number(value); is that it casts falsy values as 0 and truthy values as 1.
That means that an empty string becomes 0, which may not be what you expect.
You could make a regular expression test on the input to determine if it fits your expectations for a number.
let input = document.getElementById('input');
let result = document....
3
General review points
Use semicolons
Try to avoid Array iterator functions in favor of for of. Generally use array iterators when you want the iteration to be an expression.
Apart from that the code looks good and is an elegant solution to parsing simple markup. It can be used as a base to provide the additional functionality you mention.
Questions
"...
1
Another way to write the same thing is:
const findUser = list => list.find(u => {
if(u.subKeys) {
return findUser(u.subKeys);
}
if(checkEligible({action: u.key}) {
callback(u);
return true; // necessary to stop the loop
}
});
findUser(USERS_LIST);
Once that you only need to receive a list, check if is eligible and do something ...
2
Use DOM not markup
Your code builds DOM by building a string containing markup. Markup is intended as a transport format eg between server and client and you should avoid using markup to build page content when you have access to the DOM.
Building a table.
Generally building page content is best done using the DOM interface rather than creating a string ...
2
I don't know if there is a way to avoid the nested loop in this case because you need to do a calc that involves every inner loop index with every outer loop index.
But I think that is not necessary to calculate row * mapWidth in every iteration in the inner loop, once that both variables are available outside it
for (let row = 0; row < mapHeight; row++) {...
1
I'm not familiar with Paper so can't say anything about that. And I won't say much about the HTML, as it's obviously just a functional placeholder, except two points:
Use double quotes " consistently on all attributes.
Don't use on... attributes. Assign event listeners in the script using addEventListener.
Regarding the script:
Use let/const instead ...
2
As long as the evaluation code is only executed on the client side, you don't have to worry, because the user can just change your code if he wants to.
An example where it could be an insecurity is if the attacker can make another user evaluate his formula. If you implement a function to share the result of a calculation in a link (for example cs50.com/...
2
Performance
This is a performance only review and does not address any styling or composition.
Code examples are focused on performance with no attention given to readability, naming, or re-usability
Time complexity != performance
Time complexity is not a measure of performance. It is a measure of how performance changes as the input size changes.
Two ...
4
Your code looks good and will work well. I have made some minor improvements to improve readability and reduce redundancy.
export async function retry(fn, retries, delayMs, onFailedAttempt) {
let attempt = 0;
while (true) {
try {
return await fn();
} catch (error) {
if (onFailedAttempt) {
onFailedAttempt(error, attempt, ...
0
The output of generateMaze always will be the same each time, except for one thing, which is the backgroundColor property value of the Square component.
In this case, you don't need a function/method, which output is a maze component; you can create a method to handle the maze colors, dependent on the given parameter, e.g.:
class MainMaze extends Component {
...
1
This could be accomplished via an XOR, or boolean not equal in other words.
const people = [
{
"name": "Olivia",
"lead": true
},
{
"name": "Gary",
"lead": false
},
{
"name": "Adam",
"lead": false
}
]
const toggleLead = (name) =>
[...people].map(person =>
({
...person,
lead: ...
3
One way to solve the problem with trees
A simplified look at binary trees
the rectangles are static
[the rectangles] should never overlap
As such we can build a simple tree.
Before looking at 2d points we can look at a solution for 1d points.
Say we have the following line segments. Beginnings are inclusive ends are not.
A: 0 - 2
B: 2 - 3
C: 3 - 4
D: 4 - ...
1
I'd like to know how I could condense this into only being called once, rather than in each function.
One option would be to make a method that accepted the calendar or type and called the associated method
methods:
generateCalendar(type) {
const generateMethod = `generate${type}Calendar`
if (typeof this[generateMethod] === 'function')
...
2
A short review;
I prefer to use the function keyword
You are using libs like a global, I would pass it in the function
If you don't need both key and value, then you can just use Object.values
Building on that, you might as well use a properly named function parameter and cut the lines in two
function moveLibs(libs, done){
Object.values(libs).forEach(...
5
Its a good or bad practice, execute some async code inside class constructor?
Constructors are always synchronous and should succeed to create an object immediately. If your async call doesn't block that and you don't care about the success of your async call, then it is not a bad practice to call the function without awaiting it, and immediately .catch and ...
0
I would recommend a generic filter generator. This way the code does not need any knowledge of the shape of your input and the caller is left to decide how to handle the individual values -
function* filter(t, f)
{ if (f(t)) yield t
switch(t?.constructor)
{ case Object:
case Array:
for (const v of Object.values(t))
yield *filter(v, ...
1
General review
Generally the code is OK and problems are due to lack of experience rather than bad style or habit.
I am unfamiliar with SpreadsheetApp so you should wait in case someone that is adds another answer before you accept this answer, if so inclined.
Rather than submit this answer I took a quick look at the DOC for SpreadsheetApp (I hope I got the ...
3
Questions.
"Its a good or bad practice, execute some async code inside class constructor?"
Class constructor functions can not be async functions. Thus a object created using the constructor (class syntax) will always immediately return the object.
It then depends on what the code does you are waiting on. If that code is essential for the correct ...
3
The job of a constructor is to initialize the instance, i.e. put it in a defined state. Using (encapsulated) async code there might result in an instance that is only partially initialized. This would result in the instance being in an undefined state and thusly in code using it, that relies on the instance being fully initialized to fail unpredictably.
On ...
Top 50 recent answers are included
