Stage 7 by kostylevv · Pull Request #1 · hyperskill/smart-calculator

kostylevv · 2018-06-13T20:37:44Z

No description provided.

swsms

Reviewed

swsms · 2018-06-19T16:45:23Z

+ * The Repl class implements a simple REPL.
+ * It supports addition and substraction including unary.
+ * It calculates the expressions like these: 4 + 6 - 8, 2 - 3 - 4 and so on.
+ * \help command explains these operations


Hello! Why do you use different types of slashes for different commands? In this project, we suppose to use the forward slash like jshell does.

Hello, thank you very much for review. Instructions here https://hyperskill.org/projects/1/stages/3 requires to use \help but /exit. I just strictlly followed it) I fixed it in next commit.

thanks, it is just a typo in the stage description! In this project, we chose slash for commands because it is used by jshell (since Java 9).

swsms · 2018-06-19T16:53:56Z

+/**
+ * The Repl class implements a simple REPL.
+ * It supports addition and substraction including unary.
+ * It calculates the expressions like these: 4 + 6 - 8, 2 - 3 - 4 and so on.


Now, It cannot calculate expressions without spaces: 2+8 -> Unsupported expression.

Fixed, thanks

swsms · 2018-06-19T16:57:30Z

+          if (line.contains("/") || line.contains("\\")) {
+            switch (line.trim()) {
+              case "/help":
+                System.out.println("Program calculates the expressions like these: 4 + 6 - 8, 2 - 3 - 4 and so on. It supports both unary and binary minuses. Enter '/exit' to terminate program.");


The terminate command is not /exit in your program, but the text prints it.

swsms · 2018-06-19T17:06:14Z

+   * @param postfix arithmetic expression in postfix notation
+   * @return result of calculation
+  **/
+  private static int calculatePostfix(String[] postfix) throws IllegalArgumentException {


You do not need to specify an unchecked exception in the method declaration. Usually, they are written in the Javadoc section @throws.

fixed, thanks

swsms · 2018-06-19T17:10:38Z

+   * @param words arithmetic expression in infix notation
+   * @return expression in postfix notation
+  **/
+  private static String[] infixToPostfix(String[] words) throws IllegalArgumentException {


It is a good idea to use postfix notation here. It allows you to support multiplication, division and brackets in future.

swsms · 2018-06-19T17:13:23Z

+        String line = sc.nextLine();
+
+        if (line != null && line.length() > 0) {
+          if (line.contains("/") || line.contains("\\")) {


Maybe it would be better to skip lines containing only spaces as well.

swsms · 2018-06-19T17:15:49Z

+
+  /**
+   * Converts operations to unified form.
+   * E.g: --- = -, ++++ = +, -- = + etc.


Can you process the operations like "++-" like a simple "-"? As an example, the Python REPL does it.

swsms · 2018-06-19T17:18:05Z

+  }
+
+
+  private static String[] lineToInfix(String line) throws IllegalArgumentException {


It is good that you decompose the program into a set of methods. You can also decompose it into a set of classes to make it easy to read and develop.

I think it is a long line to read it.

I've tried to decompose, but it is still look like spaghetti code. I would be grateful for the advice on decomposition.

I would take some of the repetitive logic to a class like Utils. You can either create a separate file containing a public class, or declare it right here if it is not a public class.

public class Repl { // ... } final class Utils { private Utils() { } // methods... }

The class may contain a set of static methods which perform typical transformations of strings using regular expressions. You can also define patterns of regular expressions as static fields of this class. It is a quite popular approach.

You can also try to model the Expression as a class or interface with a single method evaluate(). Then you can extend or implement the class using a PostfixExpression with the overridden method to perform evaluation logic.

So, your main class will contain code only to read/output results.

But this is just one of many ways to decompose it as simple as possible.

By the way, we have added a new stage for this project. It uses the postfix notation like your program.

swsms · 2018-06-19T17:24:16Z

+
+    for (String word : expr) {
+      if ((!word.matches("[0-9+-]+")) || (word.contains("-") && word.contains("+"))) throw new IllegalArgumentException("Unsupported expression: " + word);
+      if (word.matches("[\\+]+[0-9]+")) {


It may be better to use Pattern and Matcher if you take a regex multiple times because it has a better performance. The matches method of a string is often used when you need just match a string only once.

swsms

Good, but there are small remarks and recommendations.

swsms · 2018-07-07T18:29:52Z

+
+			if (line != null && line.length() > 0 && line.trim().replace(" ","").length() > 0) {
+				//processing commands
+				if (line.contains("/")) {


It may be better to use startsWith(...) instead of contains(...).

swsms · 2018-07-07T18:38:00Z

+}
+
+//This class represents an expression
+class Expression {


I see you made a good decomposition! I wrote about it in the previous review.

swsms · 2018-07-07T18:43:24Z

+
+
+	/**
   * Evaluates arithmetic expression in postfix notation.


Note, there is a new stage describing this algorithm: https://hyperskill.org/projects/1/stages/21.

Implemented

We've added a new stage with large arithmetics to REPL.

There is a number of new projects as well:
Encryption-decryption: https://hyperskill.org/projects/12/
Online chat with networking: https://hyperskill.org/projects/13/

swsms · 2018-07-07T18:45:22Z

-   * @return operation in unified form: + or -
-   * @throws IllegalArgumentException if operation can not be converted to unified form
-  **/
-  private static String convertOperation (String word) throws IllegalArgumentException {


Usually, unchecked exceptions are not declared in a method declaration. But you can still catch them.

swsms · 2018-07-07T18:51:35Z

+
+			//case for "assign" type of expression e.g. "x = ..."
+			if (expAssignsValueToVariable) {
+				//extract right side of an expression


I would hide this code in a separate private method to make this method more readable and concise.

Fixed, thanks!

swsms · 2018-07-07T18:55:17Z

+				}
+			} else {
+				//substitute variables
+				substLine = substituteVariables(infixLine, variables);


I think, it is the "main" routine of this method and this code is short. You may read this topic.

swsms · 2018-07-07T18:57:41Z

+		if (operator == null) throw new IllegalArgumentException("Unsupported operator NULL");
+
+		//equals
+		Pattern pattern = Pattern.compile("^[=]*$");


You can try to move all patterns to a special class like ExUtils as well as methods that checks/parses/etc strings.

Partially implemented, thanks.

swsms · 2018-10-11T06:52:30Z

+	private static final Pattern ALLOWED_CHARS = Pattern.compile("^[a-zA-Z0-9+-=*/()^]*$");
+
+	//operator patterns
+	private static final Pattern EQUALS_PATTERN = Pattern.compile("^[=]*$");


There is a set of good regexes!

kostylevv added 4 commits June 13, 2018 23:33

The sum of two integers

0a6a8f6

The sum of mult integers

d2fb8d3

REPL with + and -

74a6058

Repl with invalid input handling

dbe5de4

swsms reviewed Jun 19, 2018

View reviewed changes

Variable support

3326b7d

swsms reviewed Jul 7, 2018

View reviewed changes

Adre13 added a commit to Adre13/repl that referenced this pull request Aug 23, 2018

Stage hyperskill#1: The sum of two integers

9bb905f

MaxHlystov added a commit to MaxHlystov/repl that referenced this pull request Sep 12, 2018

Solve task hyperskill#1

0aa1994

kostylevv added 4 commits October 1, 2018 11:26

contains -> startsWith

b536c26

Stage hyperskill#7: Multiplication, division and parentheses

90c83e0

Minor refactoring

8237412

POW added

beb0f30

kostylevv changed the title ~~The sum of two integers~~ Stage 7 Oct 5, 2018

eval() refactoring

e95cdea

swsms reviewed Oct 11, 2018

View reviewed changes

		}


		private static String[] lineToInfix(String line) throws IllegalArgumentException {

Conversation

kostylevv commented Jun 13, 2018

Uh oh!

swsms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swsms Jul 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swsms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swsms Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

swsms Jul 7, 2018 •

edited

Loading

swsms Oct 11, 2018 •

edited

Loading