Skip to main content
Commonmark migration
Source Link

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Efficiency issues

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

Overall Code Quality

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Tips for improvement

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


Efficiency issues

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


Overall Code Quality

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

Tips for improvement

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

From your previous questionyour previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.

Source Link
JvR
  • 2.8k
  • 12
  • 25

From your previous question:

I have a simple calculator program that needs some reviewing. Is this as efficient as it can get, or is there a better way?

I'll try tackling both processing speed and memory use.


#Efficiency issues#

Creating new strings often. Examples are on lines 116, 124, 127, 131, 149, 158. Use String.substring when you do need a part of your original string.

Testing boundaries with IndexOutOfBoundsException at 135 and 163. A quick profiling run with VisualVM had your code spending about 80% of its time throwing and catching exceptions in valid expressions. Do an explicit boundary check like i < s.length() instead.

Wrapping/unwrapping objects at 122, 135, 137, 139, 143, 144, 147. Consider using the == and != operators when comparing chars.

ArrayList.trimToSize() shrinks the underlying array's length to the number of elements, and is only useful for very long-lived lists or those you removed many elements from.

Repeated string parsing and pattern matching at 43 and 53. Using a more structured parsing approach would alleviate this.

Double → BigDecimal → Double mutating at multiplyOrDivide. There is little gain in doing this; consider keeping your numbers either as BigDecimals or as Doubles all the way.


#Overall Code Quality#

The items listed under efficiency issues are also code smells, so I won't repeat them here.

The exception re-use in line 35 (and thrown on 46, 56, and 70) is evil. It contaminates your method signature; it removes context and gives a misleading stack trace; it forces users of the method to create an exception instance regardless of whether or not it'll be actually thrown. If the input is illegal, throw an exception on the spot like so:

throw new IllegalArgumentException("Number expected, found: " + s);

#Tips for improvement#

Consider explicit lexical analysis. Try separating the string parsing from the calculation:

  1. Parse strings into tokens. Don't look at operator syntax yet: just slice into numbers and operators.
  2. Build a precedence tree. Here you check syntax and perhaps illegal operations (division by zero).
  3. Actual evaluation.

This will allow you to handle more or different operators, such as parentheses, negation, and so on.

Look into the Scanner class for some helpful functions, like nextBigDecimal().

Premature optimisalisation is the root of evil. First get it working, then profile your application to catch efficiency problems, and then see what you can do about any issues that ping on the radar.

Aim for bite-sized functions. getNumsAndOp is a bit of a beast. multiplyOrDivide and plusOrMinus feel just about right in size. This will also make profiling easier for both you and the HotSpot compiler.