New answers tagged javascript
0
I really like the answer by morbusg.
I would change the order to keep the data last and curry it
const partition = predicate => arr =>
arr.reduce((acc, e) => {
acc[predicate(e) ? 0 : 1].push(e)
return acc
}, [[], []])
1
Private methods
There isn't one de-facto convention about private method naming. For example, the Google JS Style guide states:
6.2.3 Method names
Method names are written in lowerCamelCase. Names for @private methods must end with a trailing underscore.1
While the AirBnB style guide states:
• 23.4 Do not use trailing or leading underscores. eslint: no-...
0
I'm pretty sure the js convention is still to prefix "private" stuff with an underscore.
class QuickSort {
sort(a) {...}
_sort(a, lo, hi) {...}
...}
The only other thing that sticks out to me is your use of ++ and -- in the while loops. While perfectly idiomatic, I think it would express the logic better to move both conditionals together in ...
2
Sadly, the PHP part is neither efficient nor optimal.
Apart from already mentioned issues, your function does too much work, which makes it non-reusable and which bloats your code in general. Remember the rule of thumb: each unit of code should mind its own business. While your function interacts with a client through $_POST and echo, connects to a database, ...
1
From a short review;
the tilescale is 16 which is a great value to multiply or divide with, so
x = x/tilescale;
z = z/tilescale;
can become
x = x >> 4; //Divide by 16 (tilescale)
z = z >> 4; //Divide by 16 (tilescale)
this does mean that changing the tilescale can become a performance drag (like in real games)
So are 256 and 4, so
return(...
0
Good things:
const is used for variables that shouldn't be re-assigned
requestAnimationFrame() is used for animating
constants and global variables are declared at the top of the script.
only three functions are used
Suggestions
avoid var unless a global variable is needed. Default to using const to avoid accidental re-assignment (which seems to be a ...
0
From a short review;
I like variables to be either spelled out or Spartan (meaning 1 char)
missedNum -> missedNumber
noDuplicates -> uniqueDuplicates?
el -> i (since you expect numbers)
Comments, you only have 1 line, and it could go ;)
Reducing loop counts is the way to reduce run time.
This counter example has 3 'loops', 1 map because I dont ...
1
I already gave many suggestions in my review of your previous question. I see that some of the advice has been incorporated - e.g. "use strict", using the spread operator, etc. Yet it seems some of the advice hasn't been used (e.g. variables starting with $, using jQuery more, etc.) but I won't lose sleep about those.
I suggest avoiding use of var ...
1
I don't think I can review all of that, but here are some thoughts to your questions and some general remarks.
const/let
I would guess this is simply that your programming style colliding with the trend in JavaScript towards functional programming where mutability is an anti-pattern and generally avoided.
The first step to using const is to avoid reusing ...
1
From a short review;
You probably want to encapsulate if(matrix === null || matrix.length === 0) into an isValidMatrix function for re-use
I would drop the whole i/j thing and go for variables named after row and col
I would collect all the rows and columns with a 0 in one set of loops, and then clear the matrix based on the collected rows and columns in ...
1
From a quick review;
You could host your templates as separate HTML files, retrievable with a fetch
You would have the overhead of extra http calls
HTML tooling might be worse for HTML templates/snippets
I agree with your boss, html code does not belong in a database
template should be const
let template = document.getElementById("vacancyTemplate&...
2
There is a more efficient way to implement getNumBits (compared to @Reinderien's answer), if you use some bitwise magic:
function getNumBits(x){
x |= x >> 1;
x |= x >> 2;
x |= x >> 4;
x |= x >> 8;
x |= x >> 16;
x -= (x >> 1) & 0x55555555;
x = ((x >> 2) & 0x33333333) + (x & 0x33333333);
...
7
For any of the following suggestions, please do profile them to test the performance difference. Performance is likely to vary across multiple browser implementations of the JavaScript interpretation engine.
Rather than a loop for getNumBits, try Math.floor(Math.log(n)/LN_2) - or maybe trunc instead of floor - where LN_2 is precomputed as Math.log(2).
Try to ...
2
Use events and callbacks to react to things as they happen instead of repeatedly checking for changes.
You can use the .then function to do whatever needs to be done once the data is received.
You can learn about the <video> element events here: https://www.w3schools.com/tags/ref_av_dom.asp
1
It would be great if the code could be simplified using dynamic imports. I haven't tried this myself but if the image names could be put into an array then something like the following might work, where the exported images are set as properties on an object that can be exported. When this SO answer to Create a loop to import files dynamically in React was ...
1
A few points, most of these I have applied to your code here: https://stackblitz.com/edit/so-in-mem-db?file=index.js
Potential use of Map and O(1) time
The javascript objects you are currently using are de-facto dictionaries, so it is unlikely you will gain anything here.
Unnecessary use of !!
You have used this frequently in your code, in all the places I ...
4
Here's my proposal:
function sortObjectByValue (obj) {
const map = new Map()
Object.entries(obj)
.sort((a, b) => obj[b][1] - obj[a][1])
.forEach(([key, value]) => {
map.set(key, value)
});
return map;
}
You don't need to create a sorted object unless you want to use it, I understand that you are using a Map so that you can ...
1
Use mergeMap on the call to getDevices, then mergeAll to flatten the results, and toArray to convert back to an array:
getCount().pipe(
mergeMap(count => range(0, Math.ceil(count / pageSize))),
mergeMap(offset => getDevices(offset, pageSize)),
mergeAll(),
toArray()
).subscribe(res => {
console.log(JSON.stringify(res));
});
See here: https:...
3
A simple reactjs form to calculate the liquidity ratio, daily cost of running, and days cash on hand
You don't need to use useEffect and result states: liquidityRatio, dailyCost. Just edit like this
When you change your inputs, the render will be called and excecute from top to bottom. And result variables will be computed again.
Notes: That just basic answer for beginner in React.
0
To Your Questions
Is there a benefit to using Classes? Right now I don't see the benefit of having a handle, like, player1 = new Player(arg,arg,etc...), player2 = new Player(arg,arg, etc...)...
Maybe there are benefits, but I can't think of any serious ones now.
The current version is similar to the factory pattern, where the creation of an object is ...
4
Neither of your solutions handles the type of the replacement correctly. For instance "bar.baz.foo:111222" replaces foo:333444 with foo:"111222" where I would expect it to result in foo:111222.
Further they can handle if the replacement is an array, but not if it is an object due to the use of split(':'). You should use str.indexOf(':') ...
2
"numeric palindromes" is rather unfortunate term. It is not clear if nonintegers are to be considered. The implementation only considers integers, in which case i would rather use term "palindromic integers".
It is also not explicitly stated, although assumed, that only base 10 representations are to be considered. Some numbers that are ...
1
You can conditionally select the wrapper instead. Use the ternary to create an internal wrapper component. Not much of a code reduction though, but rather just moving the logic a bit. You can also remove the react Fragments as all the returns are returning single react nodes, which does help reduce the code and IMO improve readability.
render() {
const ...
0
Consider what you are doing here. You are generating a list of primes less than a number. You are generating this list in order of increasing size. One simple optimization is to seed the list with a few primes at the beginning. In particular, 2 and 3. Then you iterate to skip over all the even numbers. That cuts your checks in half.
Now, a second point ...
4
This is not a proper review, but rather an extended comment.
Are there shorter, more-concise alternatives to the above approach
Yes. You should view it not as a programming problem, but as a recreational math exercise.
Consider a simplest case of N being an even power of 10, say 1000000. For a moment, allow leading zeroes. Now, each 6-digit palindromic ...
2
The function you posted seems to me to be the best. Here is your function re-arranged slightly using short-circuiting as an alternative way:
const sumOfPalindromes = n => {
let x = 0;
for (let i = 0; i <= n; i++)
i == (''+i).split('').reverse().join('') && (x += i);
return x };
console.log(sumOfPalindromes(...
0
I think I found solution much cleaner and simpler to understand:
https://codereview.stackexchange.com/a/240907/227075
That solution adjusted to my task
const testStr = 'bar.baz.foo:111222';
const testObj = {
bar: {
baz: {
foo: '333444',
},
},
};
console.log('[before] testObj.bar.baz =', testObj.bar.baz);
// run
replaceInObj(testObj, ...
2
From a lengthy review;
Clean up tags that are not used (doubles like body or unused like nav)
Use a beautifier like https://beautifier.io/
Make sure you close your tags properly (the closing tag of the time section was missing)
Leverage css styling, inline styling does not scale, and HTML is not the right place to style!
document.getElementById is supported ...
1
Your functionalmethod is fine, just the place You have defined is a bit anti pattern. (But well, if you really want to use that component in JUST in that scope its still fine.)
But let me try to refactor it in a react way:
render () {
return (
<>
{
showContainer
? (
<Modal&...
3
Let me share my async await solution for that:
Because Your fetch function is pretty similar till the res.text(), I would create an own function (fetchData ) for that, what is returning a Promise, what is resolving a res.text()
Now you can assign an async to your getLogsfunction
Lets reorganise your code above
Error handling is missing here, but take care ...
3
I'm not familiar with vue.js, but perhaps you could expand upon what & why this doesn't seem right to you. These look to be two discrete components with functions that look similar but because of the simplicity of them I can't say it's actual code duplication.
I have only a couple significant comments about each deleteX functions
deleteDirectionForm: ...
0
Update 30 June 2020 with a new Updated/Improved Version.
While maintaining the same concept of using the Single Loop String Triples (SLTS) method, the code has been updated and improved as follows:
Variables names are now more readable.
Use of let instead of var.
Removed the counter var ‘j’ to track the scale positions. Now computed from the triplet ...
1
The review will be focused on readability and testability.
Naming
I read (I think it was in Clean Code) that variables should not have a postfix like Data and Information and instead we should give things a name that they represent.
PortfolioData is actually a Portfolio while your current Portfolio is more a PortfolioRepository or PortfolioCollection.
...
1
From the array variable:symbols_list = [...'0123456789abcdefghijklmnopqrstuvwxyz'], I would assume that the function would permit the use of alternative symbols for representing numbers in formats above the decimal format. I have not come across such formats, like Hexadecimal numbers being represented by letters other than the English (Latin) letters A to F.
...
1
You should measure the duration of each step in the algorithm to detect where the bottleneck(s) is/are. You can do that using console.time("id") paired with console.timeEnd("id"):
function findPrimes(count) {
console.time("prime generation");
storePrimes(count);
console.timeEnd("prime generation")
...
0
As was already mentioned in the comments, a switch statement could be used to clean up the sets of if statements. Another option would be to abstract the similar callback functions passed to .reduce()- perhaps using partially applied functions to accept parameters - e.g. the minimum values in conditions like currentChamp.getCost() <= 3.
It seems like let ...
0
This is a great alternative to timeouts. I've added the clearTimeout for completeness.
function requestTimeout(callback, delay) {
var tickTimeout = window.requestAnimationFrame, dateNow = Date.now,
start = dateNow(),
stop = false,
timeout = function () {
if (dateNow() - start < delay) {
return ...
4
calc1 and calc2 are really bad names. Names should always clearly describe what they are for or do. calculateCircleCircumference and calculateCircleArea would be better
choices.
I'm not a big fan of the names commaToDot and commaToDot either. I'd prefer more conceptional names such as parseNumber and formatNumber, maybe even parseGermanNumber and ...
1
Improvement in function isprime:
for(let i = 2; i <= number / 2; i++)
can be
for(let i = 2; i <= Math.round(Math.sqrt(number)) + 1 ; i++)
Otherwise, the best easy to understand approach(in accordance to my knowledge) is to use the Sieve of Eratosthenes. Your problem can be a subset of the following problem Sieve of Eratosthenes JavaScript ...
0
Note- the review below only pertains to the code in the post. "Code that's not included can not be reviewed."1
Overall I would say this code is well-written. Variables are declared with const and let appropriately. It has good documentation - especially for each method- although example usage is tough to find. One possible change you could consider ...
3
If my form grows .. I'd have to add state ... and add state ... and [add] another case to my switch statement
This is a code smell, and not unique to Javascript. This is design problem and the fix has to be at the core of the design - a JS data structure will fix it.
An array of field objects (in the proper order) will eliminate code proliferation. You will ...
3
Looks to be fairly clean code.
Suggestions
Use string::slice or string::substring versus string::substr. substr isn't deprecated but also isn't recommended for use.
Warning: Although String.prototype.substr(…) is not strictly
deprecated (as in "removed from the Web standards"), it is considered
a legacy function and should be avoided when ...
4
As this question was asked nearly four years ago it is likely the case that you have learned more about JavaScript since then and/or changed this code. Nonetheless perhaps the info below will help you and/or somebody else.
It is interesting that The BackBoneJS documentation uses some valid JSON for specifying the keys of objects but not all. The keys in the ...
2
Just in my opinion, take this with grain of salt.
You don't need that at all:
$return = [
"email" => "",
"password" => "",
"error" => false
];
Ajax response data type could be just a text, one of these: [bad credentials, error, success], you should validate if email or password is empty before ...
2
There is nothing wrong with your code when changing the data size to a larger number, except that the draw() method doesn't adjust the width of the columns by the ratio between the canvas size and the number of columns to be drawn.
You'll have to do some math like:
let offset = Math.round(canvaswidth / len);
let rectWidth = Math.round(offset * 0.9);
The ...
0
Other than your handleOnChange handler is mutating state, this code seems understandable and readable enough to me, only minor comments about code style.
React-ness
React's synthetic events' persist function is a void return, so I don't think the logical OR between it and the state update necessary.
Mutating-state handler with odd event persisting.
...
0
In regards to readability, I agree that there is room for improvement. It seems like you're trying to cram as much code in as you can in fewer lines.
Use the conventional style for Javascript class methods where you have `
name_of_method(arg1, arg2) {
return ...
}
instead of setting them like you are now, like you would a local variable. Additionally, ...
3
First and foremost, I don't really like the idea of an ajax login because imagine the user is on a public device and logs in then steps away from the computer and someone else pulls up to the browser, opens up the Developer tools > Network > XHR > Headers and gets an eyeful of the login credentials that were sent off for logging in. Is it a bit &...
1
I don't know enough about the shape of testData to write great code, but you could preprocess and convert testData from an array to an object or Map of key-value pairs. By using the first element of each item in testData as the key, then you can convert your \$O(n)\$ inner "search" loop into a constant time \$O(1)\$ key lookup. This would reduce ...
1
Return Value
The method addUser returns false for all negative cases. Below it is possible to see that it does not return true for the positive case.. instead it returns nothing which leads to an undefined:
else
{
$('#loadingDiv').show();
$.post('process/editUser.php', {uobj:uobj}, function(data)
{
var modal = "#addUserModal";
$('#...
Top 50 recent answers are included
