New answers tagged javascript
3
First of all I would recommend more care formatting your code and choosing good names for your variables and functions. findingNeibors function misspells neighbors and doesn't find them, it gives a sum of all the neighbors in a matrix, so I renamed it sumMatrixNeighbors. You can see the other changes on the code below. The i and j are conventionally used for ...
4
Don't return if there is an error, throw the error.
if(i<0 || j< 0) {
throw new Error("Invalid Index");
}
if(i>rowLimit || j> columnLimit){
throw new Error("Index out of bounds");
}
Don't assume an array has elements
(i.e.: let columnLimit= myArray[0].length-1; will throw a TypeError if there are no elements in the list)
Play it safe....
1
If you want to use object-oriented javascript for this calculator, I would prefer the class syntax over the prototype syntax.
This prototype approach ..
function Calculate(num1, num2, op) {
this.num1 = num1;
this.num2 = num2;
this.op = op;
}
Calculate.prototype.result = function() {
// ..
}
.. can be replaced by (...
1
I don't really see any purpose to forcibly using a class setup here. All that's doing is allowing you to provide the data before result is called. That's not necessary here though, and there are other ways of achieving that. If you needed to create multiple Calculate objects, and retain them, and call result on them multiple times, there may be a purpose. I ...
1
Here are a list of improvements:
Use ECMAScript features and syntax
Create own implementation of BigInt
Don't use strings !
Create functions to regroup similar code and reduce code duplication
Detailed Explanation:
Generates a ListNode from a given array (array should already be reversed).
generateListNode = (list) => {
const [ele] = list;
//create ...
0
Does the longer version have a better performance?
No, the longer version has two different if conditions and you're recalculating the scroll area twice (i.e.: document.body.scrollTop > 100 || document.documentElement.scrollTop > 100 and document.body.scrollTop <= 100 || document.documentElement.scrollTop <= 100)
Does the browser optimize it ...
2
if (!isNaN(str1[i]) && !isNaN(str2[i])) { // if both chars are numbers, return str with numerically smaller number
return parseInt(str1.slice(i)) < parseInt(str2.slice(i)) ? str1 : str2;
}
seems like a bug. In case the numbers compare equal the code blindly returns the first string. Consider (a10c, a10b).
return str1; // if we ...
3
I would not do this. It breaks too far from any typical convention for very little gain.
I'm personally against omitting braces at all; except in very narrow circumstances. I've seen many questions on stack overflow that stem from people omitting braces and not being able to figure out why "only certain lines are running".
This also appears to look like a ...
2
You have a question embedded in the code:
void(set.delete(char) || set.add(char)); // <--is this good code?
The void operator "evaluates the given expression and then returns undefined"1. It doesn't appear that there is a need to have undefined be the final value of that expression because that expression doesn't get assigned or returned. The void() can ...
1
General feedback
The code is quite redundant so to adhere to the Don't Repeat Yourself principle, there are a couple things that can be done:
Abstract common lines from functions like reset() and initialize() into a separate function that can be called in both places. That way if you need to update logic it can be done in one place instead of multiple.
...
3
Here's my answer. Note I did not touch the html. We could attack both the javascript and the html and condense them both, but I think that just one file change is necessary right now.
$(document).ready(function () {
// Global variables
var targetNumber;
var userTotal = 0;
var wins = 0;
var losses = 0;
// Functions
function ...
3
Use toggle methods
Presuming that you are just toggling those two classes, you could just use classList.toggle() with those two class names:
function toggle_expandable_icon(elem) {
elem.classList.toggle('fa-plus-circle');
elem.classList.toggle('fa-minus-circle');
}
And it appears jQuery is used, so the .toggleClass() method could be used on a ...
0
// const fs = require('fs');
// const folder = "../api/";
const regex = /^v[0-9](\.[0-9]{1,2})?(\.[0-9])?$/;
// let files = fs.readdirSync(folder);
let pattern = "v5.2"; // let pattern = req.params.v;
var arr = ['v1.0.0', 'v5.2.4', 'v5.2.9', 'v5.20.4', 'v6.4.0']; // var arr = [];
/** Function **/
/*
function naturalSort(a, b) {
var ax = [],
bx ...
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 ...
-1
const Login = () => (
<LayoutNoWrap>
Login
</LayoutNoWrap>
);
export default Login;
2
General points
I think that multi line declarations are poor practice and can easily lead to global declarations if you have a miss placed ; and don't use the "use strict" directive.
As the DOM interface is rather verbose you can create some helper functions to reduce the amount of noise in your code and save yourself the trouble of having to repeat the ...
2
I'd recommend looking into using a <template> tag, and possibly templates with slots unless the browser compatibility causes an issue. That way the template can remain in the markup, and all the Javascript logic might need to do is update the content that is dynamic (e.g. the caption text).
Instead of using setAttribute() to add a class name to ...
5
Your code looks good and concise.
Your test cases cover almost every branch of the code, which is also good.
You can improve the variable names:
The temp in tempDocument is not necessary. Therefore the variable name should better be document, or if that is too long, doc. Be careful though when abbreviating variable names, and don't make them ambiguous.
...
3
Not a comment on functionality, but here:
const wideImg = document.querySelector('.full-width-image-atf img'),
captionText = wideImg.getAttribute('alt'),
captionInnerHTML = '<span class="image-caption">' + captionText + '</span>',
caption = document.createElement('div');
I don't think chaining declarations using , like you are ...
1
Build reusable
Always try to code in such a way that the effort you put in can be reused, first in the same app, and maybe in other apps.
One would assume that you likely have more than one list that acts like radio group, or even unrelated DOM items as part of a radio group.
Your best bet is to create a simple object that tracks the state of a group and ...
1
Another approach could be with hidden radio inputs and :checked CSS selector :
input { display: none }
input:checked ~ * { color: red }
<label><input type="radio" name="li" checked><li>1</li></label>
<label><input type="radio" name="li"><li>2</li></label>
<label><input type="radio" ...
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 ...
1
Make it faster
This is only improvements on the algorithm you have used.
There are a few ways to improve the performance. The majority are minor a few % points improvement but due to the complexity ~\$O(3^n)\$ quickly sum up to be worth the effort.
The last step
There is a very common mistake in recursive code that will nearly always give a huge ...
0
You could use some newer methods like spread syntax in array and forEach loop on node list for this. You could also write separate function to get sibling nodes and to toggle class based on sibling nodes.
This way you can toggle class on one element and also toggle based on other element nodes.
const li = document.querySelectorAll('li');
function ...
2
I see a couple of simplifications that could be made:
A feature of ecmascript-6 that could be used here is default parameters:
const combine = (cur = '', n = 0) => {
Then the first call to that function doesn't need to pass any parameters:
combine();
The arrow function in the imperative approach feels too brief to warrant the need for brackets:
[......
1
First of all, ES2015+ onwards have template literals forw easier interpolation of string and data. For instance:
function fix_m_link(){
var item1 = '@'
var item2 = '.'
var m_clear_text = `test${item1}example${item2}com`
document.getElementById('m_link').innerHTML = `<a href="mailto:${m_clear_text}">${m_clear_text}</a>`
}
Also, ...
0
Disclaimer: I´m not an expert in redux / redux-saga. This post is not directly answering your questions. Rather, it is focused on code design / architecture and what is the most useful approach for the underlying problem.
For me, the main question is: Who is in charge of determining when the data is invalid?
Imho, there are at least three answers to this:
...
3
Names
The plural of parenthesis is parentheses (as used in the specification). So I think it would make more sense to call the top level function generateParentheses, or even generateBalancedParentheses.
The inner function should be addParenthesis (with an e substituted for the third a).
Opinions will vary on cur as an abbreviation for current. I think it'...
4
I think your functional approach is very nice, and scores high points for readability. As long as the recursion wasn't causing performance issues, that's how I'd approach it.
However, it's not very efficient, and as vnp points out, listing permutations is really just a matter of counting from 1 to "the number of combinations" in a mixed base numbering ...
8
whenever you encounter a task where you have to combine or permute things, then backtracking is a possible approach.
True. It is not necessarily the best though (in fact it is rarely the best).
In this particular case, the problem naturally maps to finding the next combination. In turn, it is no more than an increment of a number in some fancy numbering ...
1
I am working on an app that needs such functionality also. So after much effort to get a solution I arrived at this.
https://stackblitz.com/edit/angular-e4ugma
import { Component } from '@angular/core';
import { BehaviorSubject } from 'rxjs';
import { Observable } from 'rxjs';
import { tap, switchMap,first } from 'rxjs/operators';
import 'rxjs/add/...
6
Performance
The main reason for the slow down are the state changes you incur each time you change the color and transparency of the particle (which is each particle). Unless you are prepared to write some shaders (webGL) you are not going to get past this problem. You will need to sacrifice some features to get performance.
The second reason for the ...
2
You might be able to improve performance by removing particles once they have left the boundaries of the screen. This should reduce your particle count substantially.
Another option to pursue would be to use getFrameRate to check your current frame rate against a target value and reduce the number of particles you generate based on that.
3
-- adding this as an answer because I can't comment.
The answer referenced in ggorlen's comment, can be improved by remarking that a (negative number % 10) is a negative number, so there is no need for sign checking.
const reverse = val => {
let res = 0;
const Base = 10;
while (val) {
res = res * Base + (val % Base);
val = (...
1
A couple of minor remarks
The active layout is used in various locations. Perhaps you should create a getter for it to avoid redundant code.
let activeLayout = getRoot(self).stage.activeLayout;
Writing in a fluent style is a delight for the eye, but it should not come at too much cost in code quality. getParent(self, 2) should have been set in a local ...
1
Sometimes it is worth generalizing an algorithm to handle an arbitrary number of inputs. Not always, of course, but at least the exercise of thinking about how you would generalize an algorithm can force you to consider whether there is any unnecessary code duplication---this might be the algorithmic analogue of looking for magic constants.
It turns out ...
1
Response to your prompt
Due to my lack of knowledge in JS I would like to know if this is a proper solution.
This code sets attributes on the elements to store properties. While this works, it technically leads to invalid HTML1. Alternatives include using data attributes and setting properties on the variable representing the element. See responses to ...
4
An easier way is to use JavaScript's Array.includes, Array.some and/or Array.find methods
let arr1 = [1, 3, 11, 32, 44, 99]
let arr2 = [4, 12, 15, 99]
let arr3 = [4, 11, 13, 15, 23, 43]
function searchThreeSameNum (arr1, arr2, arr3) {
return arr1.find(number => {
return arr2.includes(number) && arr3.includes(number)
})
}
const result = ...
3
Well, you have one huge bug in the code. You have return … as the last line of the while(1) loop. This return will be hit the moment first arr1 and arr2 elements match but arr3 does not contain that element. I suggest you to use Roland's approach as it works and keeps your general algorithm the same.
1
May i suggest a shorter code?
var data = {
dataGroup: {
count: [4, 4, 2, 1],
dataOne: [1, 3, 2, 1],
dataTwo: [7, -1, 9, 0],
dataThree: [3, 8, 1, 2]
}
}
var processedData = Object.keys(data.dataGroup).filter(key => key != 'count').reduce((acc, key) => {
acc.groupName[key] = [];
data.dataGroup.count.forEach((amount, ...
3
Since you're iterating for the sole purpose of creating an array, map is somewhat appropriate here.
const productArr = [... Array(dataLength)] // Create a dummy array dataLength elements long
.map(_ => ({ // And map over it
name: faker.commerce.productName(),
category: faker.commerce....
5
Foreword
There is quite a bit of code here and it has taken a bit of time to process - especially the unit tests. I feel like the thoughts below will be incomplete but if I think of other ideas in the future, I could add those later.
General Feedback
I know you have made various versions of this in the past - including a jQuery version posted in 2014 and ...
14
Your code assumes that each of the 3 arrays is sorted. Otherwise the < operator would not work. It's ok to assume this. You should have mentioned this in your question.
You use the == operator for comparing the numbers and the lengths. You should better use the === since the == operator considers 0 and "0" equal, which is not good in most cases.
It does ...
10
Although your code seems to work, it is difficult to read. Loops inside loops and many if, else if blocks and continue or return. Let's start with some big issues:
You use an endless while loop when it is clear you don't have to. The code below would perform the same function:
function searchThreeSameNum(arr1, arr2, arr3) {
let i = 0, j = 0;
while (...
1
I liked the idea of doing this in a way which is analogous to the way the image is drawn, that is to say, creating a data structure which is somewhat similar to the image you have above, and then using that to count which spaces would hold water.
This isn't so much of a code review, as a presentation of an alternative way of doing the same thing (as are ...
4
I stumbled upon your question by accident (I normally only visit stackoverflow) and finally decided to make an account.
The Idea
After I saw @Blindman67 answer (which I cannot comment since I do not have enough reputation), I was wondering if we could reduce the iterations from n∗1.5 to just n. Thus, I propose this idea. Instead of evaluating all the way ...
0
Responding to your prompt
Any good suggestions to avoid HTML code in controller make controller view and script separate.
I would suggest you consider using a template on the client-side, and having the AJAX request return data in JSON format. That way the Laravel controller doesn't have to worry about the HTML formatted-data.
Review Feedback
That name ...
0
This is my improved version.
it builds on top of isEmpty()
correctly treats undefined in my test cases (tricky as undefined is not an object unlike null)
isDeepEmpty([0]) = true and return values for other test cases are pretty intuitive
isDeepEmpty(42) = false, but isEmpty(42) behaves the same way
Here:
import isEmpty from 'lodash/fp/isEmpty'
export ...
3
Your example:
public function ajax(Request $request){
$data['products'] = Product::select('products.id', 'products.name', 'products.banner')->get();
foreach ($data['products'] as $product){
echo $product->name;
}
Attn: I omit commented string.
You do not need to get the query results into $data, especially if before you perform $data = $...
2
Reusable
One of programmings main attributes it the ability to reuse code. You should always write code with this in mind
When you create code think about how you may need to do the same thing over and over. Write a function to do that task in such a way so that you can add it to a library. This will reduce the amount of work needed to complete the current ...
Top 50 recent answers are included
