-
Notifications
You must be signed in to change notification settings - Fork 18
Stage 7 #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Stage 7 #1
Changes from 4 commits
0a6a8f6
d2fb8d3
74a6058
dbe5de4
3326b7d
b536c26
90c83e0
8237412
beb0f30
e95cdea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| import java.util.*; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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). |
||
| * /exit command terminates application | ||
| */ | ||
| public class Repl { | ||
| public static void main(String[] args) { | ||
| boolean cont = true; | ||
| while (cont) { | ||
| Scanner sc = new Scanner(System.in); | ||
| String line = sc.nextLine(); | ||
|
|
||
| if (line != null && line.length() > 0) { | ||
| if (line.contains("/") || line.contains("\\")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it would be better to skip lines containing only spaces as well.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| 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."); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The terminate command is not /exit in your program, but the text prints it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| break; | ||
| case "\\exit": | ||
| cont = false; | ||
| break; | ||
| default: | ||
| System.out.println("Unsupported command"); | ||
| } // | ||
| } else { | ||
| try { | ||
| String[] postfix = infixToPostfix(lineToInfix(line)); | ||
| int res = calculatePostfix(postfix); | ||
| System.out.println("Result = " + res); | ||
| } catch (IllegalArgumentException e) { | ||
| System.out.println(e.getMessage()); | ||
| } | ||
| } | ||
| }//eof if | ||
| }//eof while | ||
| System.out.println("Bye!"); | ||
| }//eof main | ||
|
|
||
| /** | ||
| * Evaluates arithmetic expression in postfix notation. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note, there is a new stage describing this algorithm: https://hyperskill.org/projects/1/stages/21.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've added a new stage with large arithmetics to REPL. There is a number of new projects as well: |
||
| * Simplified Dijkstra algorithm: supports only + and - operations | ||
| * | ||
| * @param postfix arithmetic expression in postfix notation | ||
| * @return result of calculation | ||
| **/ | ||
| private static int calculatePostfix(String[] postfix) throws IllegalArgumentException { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You do not need to specify an unchecked exception in the method declaration. Usually, they are written in the Javadoc section @throws.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed, thanks |
||
| if (postfix != null && postfix.length > 0) { | ||
| Deque<Integer> res = new LinkedList<Integer>(); //results stack | ||
| res.addFirst(0); //add dummy value to allow unary operations | ||
| for (String word : postfix) { | ||
| if (isOperand(word)) { | ||
| res.addFirst(Integer.parseInt(word)); | ||
| } else if ((word.contains("-") || word.contains("+")) && res.size() >= 2) { | ||
| int operand1 = (Integer) res.removeFirst(); | ||
| int operand2 = (Integer) res.removeFirst(); | ||
| int result = 0; | ||
| if (word.equals("-")) { | ||
| result = operand2 - operand1; | ||
| } else if (word.equals("+")) { | ||
| result = operand1 + operand2; | ||
| } else throw new IllegalArgumentException("Unsupported operation " + word); | ||
| res.addFirst(result); | ||
| } else throw new IllegalArgumentException("Can't process an expression"); | ||
| } | ||
| return (Integer) res.removeFirst(); | ||
| } else throw new IllegalArgumentException("Expression is null or zero length"); | ||
| } | ||
|
|
||
| /** | ||
| * Converts infix expression to postfix notation. | ||
| * | ||
| * @param words arithmetic expression in infix notation | ||
| * @return expression in postfix notation | ||
| **/ | ||
| private static String[] infixToPostfix(String[] words) throws IllegalArgumentException { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a good idea to use postfix notation here. It allows you to support multiplication, division and brackets in future. |
||
| if (words != null && words.length > 0) { | ||
| String[] result = new String[words.length]; | ||
| int i = 0; | ||
| Deque<String> stack = new LinkedList<String>(); | ||
| boolean prevIsOperation = true; | ||
| for (String word : words){ | ||
| if (word.matches("[\\+]+") || word.matches("[-]+")) { | ||
| String operation = convertOperation(word); | ||
| if (stack.size() > 0) { | ||
| result[i++] = (String)stack.removeFirst(); | ||
| } | ||
| stack.addFirst(operation); | ||
| prevIsOperation = true; | ||
| } else if (isOperand(word) && prevIsOperation) { | ||
| result[i++] = word; | ||
| prevIsOperation = false; | ||
| } else throw new IllegalArgumentException("Unsupported expression"); | ||
| } | ||
| if (stack.size() > 0) { | ||
| result[i++] = (String)stack.removeFirst(); | ||
| } | ||
| return result; | ||
| } else throw new IllegalArgumentException("Passed array is null or empty"); | ||
| } | ||
|
|
||
| /** | ||
| * Checks wheter String is operand (int value) | ||
| * @param word string to check | ||
| * @return true if provided string can be converted to int, false otherwise | ||
| **/ | ||
| private static boolean isOperand (String word) { | ||
| if (word != null) { | ||
| try { | ||
| Integer.parseInt(word); | ||
| } catch (NumberFormatException e) { | ||
| return false; | ||
| } | ||
| } else return false; | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Converts operations to unified form. | ||
| * E.g: --- = -, ++++ = +, -- = + etc. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you process the operations like "++-" like a simple "-"? As an example, the Python REPL does it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now it can |
||
| * @param word operation in raw form | ||
| * @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 { | ||
| String plusPattern = "[^\\+]"; | ||
| Pattern plus = Pattern.compile(plusPattern); | ||
| String minusPattern = "[^\\-]"; | ||
| Pattern minus = Pattern.compile(minusPattern); | ||
| Matcher plusMatcher = plus.matcher(word); | ||
| Matcher minusMatcher = minus.matcher(word); | ||
| if (!plusMatcher.find()) { | ||
| return "+"; | ||
| } else if (!minusMatcher.find()) { | ||
| if (word.length() % 2 == 0) return "+"; | ||
| else return "-"; | ||
| } else { | ||
| throw new IllegalArgumentException("Unsupported operation: " + word); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| private static String[] lineToInfix(String line) throws IllegalArgumentException { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is a long line to read it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tried to decompose, but it is still look like spaghetti code. I would be grateful for the advice on decomposition.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. 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. |
||
| String[] expr = line.replaceAll("\\s+"," ").split(DELIM); | ||
| String[] result = new String[expr.length]; | ||
| int i = 0; | ||
|
|
||
| for (String word : expr) { | ||
| if ((!word.matches("[0-9+-]+")) || (word.contains("-") && word.contains("+"))) throw new IllegalArgumentException("Unsupported expression: " + word); | ||
| if (word.matches("[\\+]+[0-9]+")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| word = word.replaceAll("\\+",""); | ||
| } else if (word.matches("[-]+[0-9]+")){ | ||
| Pattern pattern = Pattern.compile("\\-"); | ||
| Matcher matcher = pattern.matcher(word); | ||
| int count = 0; | ||
| while (matcher.find()) { | ||
| count++; | ||
| } | ||
| if (count % 2 != 0) { | ||
| word = "-" + word.replaceAll("\\-",""); | ||
| } else { | ||
| word = word.replaceAll("\\-",""); | ||
| } | ||
| } | ||
| result[i++] = word; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| private static final String DELIM = " "; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, It cannot calculate expressions without spaces: 2+8 -> Unsupported expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks