Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Feedback

#Feedback II like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

Suggestions

##Javascript

Javascript

Data format

###Data format InsteadInstead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

Cache DOM references

###Cache DOM references ThereThere are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

if chains (Javascript)

###if chains (Javascript) YouYou mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

let vs const

###let vs const InIn the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

For more context, refer to the article below. I know it comes off in the beginning as tough on jQuery but it has some useful information. Stop writing Slow Javascript.

PHP

Unused variable: $status

##PHP ###Unused variable: $status ItIt appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

Mapping built-in functions

###Mapping built-in functions TheThe static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

For more context, refer to the article below. I know it comes off in the beginning as tough on jQuery but it has some useful information. Stop writing Slow Javascript.

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

Feedback

I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

Suggestions

Javascript

Data format

Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

Cache DOM references

There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

if chains (Javascript)

You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

let vs const

In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

For more context, refer to the article below. I know it comes off in the beginning as tough on jQuery but it has some useful information. Stop writing Slow Javascript.

PHP

Unused variable: $status

It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

Mapping built-in functions

The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';
Add reference for JS optimization
Source Link

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

For more context, refer to the article below. I know it comes off in the beginning as tough on jQuery but it has some useful information. Stop writing Slow Javascript .

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

For more context, refer to the article below. I know it comes off in the beginning as tough on jQuery but it has some useful information. Stop writing Slow Javascript .

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';
Update wording
Source Link

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (likeand as mentioned above, the value doesn't appear to get re-assigned so const can be used).

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (like mentioned above, the value doesn't appear to get re-assigned so const can be used).

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';

#Feedback I like the usage of an event delegate in the JavaScript code - it is nice how the click event is bound once. Also, the event handling code is separated from the markup - great job!

#Suggestions

##Javascript

###Data format Instead of using substrings to separate the operands and operator, String.split() could be used to separate the expression into an array of pieces. Actually, maybe it would be wise to store the expression using that data structure and display it by by calling Array.join() on the array from calling Object.values().

###Cache DOM references There are multiple places where $('#Expression') exists - that could be stored in a variable when the DOM is ready (e.g. passed to the Expression constructor function, which would store it on this) and then that variable referenced instead of fetching it each time.

###if chains (Javascript) You mentioned (when discussing the PHP code) that you don't like long if chains. But it appears that there is a decent if chain in the Javascript code that delegates function calls based on the button that was clicked. You could likely simplify that by creating a mapping of buttonText values to functions of Expression

###let vs const In the function Expression.prototype.Submit, there is a variable Data declared using let. It appears that value doesn't change within the lifetime of that function. Why not use const instead? The same applies to buttonText in the click handler.

###jQuery wrapper on event

The line:

let buttonText = $(e).attr('target').innerHTML;

Could be simplified to merely:

const buttonText = e.target.innerHTML;

Because there isn't much need in passing the event object in the jQuery wrapper just to get the innerHTML of the target (and as mentioned above, the value doesn't appear to get re-assigned so const can be used).

##PHP ###Unused variable: $status It appears that variable $status is unused. Was that used for debugging, or checking the value of $code?

###Mapping built-in functions The static anonymous that call built-in functions (e.g. min(), max(), pow()) could just be replaced by the name of the function in a string literal.

$OperationMap['^'] = 'pow';
$OperationMap['⋀'] = 'max';
$OperationMap['∨'] = 'min';
update explanation
Source Link
Loading
update explanation
Source Link
Loading
Mention array
Source Link
Loading
mention const
Source Link
Loading
Source Link
Loading
lang-php

AltStyle によって変換されたページ (->オリジナル) /