Siarhiej Kresik #32

siarhiejkresik · 2019-05-10T16:14:15Z

Description

A command-line calculator implemented using Top Down Operator Precedence parsing algorithm (Pratt parser).

Minimal requirements

Calculator should be a command-line utility which receives mathematical
expression string as an argument and prints evaluated result:

$ pycalc '2+2*2'
6

It should provide the following interface:

$ pycalc --help
usage: pycalc [-h] EXPRESSION

Pure-python command-line calculator.

positional arguments:
  EXPRESSION            expression string to evaluate

In case of any mistakes in the expression utility should print human-readable
error explanation with "ERROR: " prefix and exit with non-zero exit code:

$ pycalc '15*(25+1'
ERROR: syntax error
15*(25+1
        ^
$ pycalc 'func'
ERROR: syntax error
func
^
$ pycalc '10 + 1/0 -3'
ERROR: division by zero
10 + 1/0 -3
      ^
$ pycalc '1 + sin(1,2) - 2'
ERROR: sin() takes exactly one argument (2 given)
1 + sin(1,2) - 2
    ^
$ pycalc '10^10^10'
ERROR: math range error
10^10^10
  ^
$ pycalc  '(-1)^0.5'
ERROR: math domain error
(-1)^0.5
    ^
$ pycalc ''
ERROR: empty expression provided

$ pycalc '1514' -m жыве calendar беларусь time
ERROR: no module(s) named жыве, беларусь

$ pycalc '2+3'
5
$ echo $?
0

$ pycalc '2+'
ERROR: syntax error
2+
  ^
$ echo $?
1

Mathematical operations calculator must support

Arithmetic (+, -, *, /, //, %, ^) (^ is a power).
Comparison (<, <=, ==, !=, >=, >).
2 built-in python functions: abs and round.
All functions and constants from standard python module math (trigonometry, logarithms, etc.).

$ pycalc 'e + pi + tau'
12.143059789228424


$ pycalc '1 + inf'
inf

$ pycalc '1 - inf'
-inf

$ pycalc 'inf - inf'
nan

$ pycalc 'nan == nan'
False

Non-functional requirements

It is mandatory to use argparse module.
Codebase must be covered with unit tests with at least 70% coverage.
Usage of external modules is prohibited (python standard library only).
Usage of eval and exec is prohibited.
Usage of ast and tokenize modules is prohibited.

Distribution

Utility should be wrapped into distribution package with setuptools.
This package should export CLI utility named pycalc.

Codestyle

Docstrings are mandatory for all methods, classes, functions and modules.
Code must correspond to pep8 (use pycodestyle utility for self-check).
You can set line length up to 120 symbols.

Optional requirements

These requirement is not mandatory for implementation, but you can get more points for it.

Support of functions and constants from the modules provided with -m or --use-modules command-line option.
This option should accept names of the modules which are accessible via
python standard module search paths.
Functions and constants from user defined modules have higher priority in case of name conflict then stuff from math module or built-in functions. (bug: -m <module_name> won’t override built-in or standard <module_name>)

In this case pycalc utility should provide the following interface:

$ pycalc --help
usage: pycalc [-h] EXPRESSION [-m MODULE [MODULE ...]]

Pure-python command-line calculator.

positional arguments:
  EXPRESSION            expression string to evaluate

optional arguments:
  -h, --help            show this help message and exit
  -m MODULE [MODULE ...], --use-modules MODULE [MODULE ...]
                        additional modules to use

Examples:

# my_module.py
def sin(number):
    return 42

$ pycalc 'sin(pi/2)'
1.0
$ pycalc 'sin(pi/2)' -m my_module
42
$ pycalc 'THURSDAY' -m calendar
3
$ pycalc 'sin(pi/2) - THURSDAY * 10' -m my_module calendar
12

AlexeiBuzuma · 2019-06-12T12:48:53Z

final_task/Makefile

@@ -0,0 +1,34 @@
+.DEFAULT_GOAL := run


С моей точки зрения в данном случае можно было обойтись без makefile.

Мне так было зручней ставіць/выдаляць праграму, правяраць, як яна працуе, запускаць pycodestyle.

AlexeiBuzuma · 2019-06-12T12:50:39Z

final_task/pycalc/args.py

+    }
+}
+
+MODULE = {


Я не вижу большого смысла в вынесении параметров для ArgumentParser в отдельные глобальные словари. Их можно было разместить прямо в аргументах функции.

AlexeiBuzuma · 2019-06-12T12:54:17Z

final_task/pycalc/calculator/importer.py

+def is_numeric(obj) -> bool:
+    """Return `True` if a object is one of numeric types."""
+
+    return isinstance(obj, (NUMERIC_TYPES))


Для чего NUMERIC_TYPES обернут в скобочки?

Яны там лішнія. Засталіся, відаць, пасля таго, як вынес tuple ў канстанту.

AlexeiBuzuma · 2019-06-12T13:20:56Z

final_task/pycalc/calculator/parselets/__init__.py

+from .punctuation import Comma
+
+
+__all__ = [


__all__ содержит все значения, которые есть в этом модуле. То есть в целом нет смысла определения этой переменной

AlexeiBuzuma · 2019-06-12T13:21:40Z

final_task/pycalc/calculator/parselets/constant.py

+        self.const_registry = const_registry
+
+    def nud(self, parser, token):
+        """"""


Пустой докстринг

Я так і не прыдумаў, як гэтыя штукі абазваць :D

AlexeiBuzuma · 2019-06-12T13:30:58Z

final_task/pycalc/cli.py

+
+        try:
+            self.calculator = calculator(modules)
+            return


В данном случае return можно опустить

AlexeiBuzuma · 2019-06-12T13:33:42Z

final_task/pycalc/cli.py

+        except Exception as exc:
+            err_msg = str(exc)
+
+        self.on_error(err_msg)


Очень странный подход, который сильно запутывает.
В данном случае получается, что последняя строчка функции исполняется только в случае если было отловлено исключение, а при нормальном исполнении функции до этой строчки исполнение не дойдет.

Это мелочь, но лично для меня это смотрится странно

Але па-другому б у канцы кожнага except прыйшлося б пісаць self.on_error(err_msg).

AlexeiBuzuma · 2019-06-12T13:35:15Z

final_task/pycalc/cli.py

+
+        self.on_error(err_msg)
+
+    def on_success(self, result):


Я думаю, что такие однострочные методы только усложняют код.
В данном случае этот метод вызывается только в одном месте и все, что он делает, это проксирует вызов другому методу. То есть фактически он ничего не делает.

З аднаго боку, яно, вядома ж, і так. Але скажам, заўтра вам скажуць, што калькулятр мае пісаць логі і пасылаць рэзультат/памылку на сервер. І калі для памылкі ёсць куды засунуць логіку, то для рэзультата трэба будзе разбірацца ў кодзе і думаць, куды яе напісаць.

AlexeiBuzuma · 2019-06-12T13:36:55Z

final_task/pycalc/cli.py

+
+        sys.exit()
+
+    def prefix_err_msg(self, msg):


Аналогичная ситуация. Метод вызывается только в одном месте и делает только конкатенацию двух строк.

Функцыя прэфіксатар робіць роўна тое, што і трэба — дадае прэфік ERROR:.
Лёгка тэставаць, лёгка мяняць, адразу відаць, што робіць, не мяшаецца з другім кодам.
Другая рэч, што можа ёй ня месца ў класе, але гэта заўсёды праблема — куды засунуць падобную функцыю, калі яна адна-дзве такія.

AlexeiBuzuma · 2019-06-12T13:39:15Z

final_task/pycalc/importer/importer.py

+UNDERSCORE = '_'
+
+
+def iter_uniq(iterables):


Эту же функциональность делает set -- хранит уникальные значения.

AlexeiBuzuma · 2019-06-12T13:41:34Z

final_task/pycalc/matcher/helpers.py

+import re
+
+
+def list_sorted_by_length(iterable) -> list:


Функция вызывается только в одном месте и делает ровно одно действите -- сортировку с помощью стандартной функции. Можно было обойтись прямым вызовом sorted

AlexeiBuzuma · 2019-06-12T13:42:08Z

final_task/pycalc/matcher/helpers.py

+    return sorted(iterable, key=len, reverse=True)
+
+
+def construct_literals_list(literals):


Такая же ситуация

AlexeiBuzuma · 2019-06-12T13:45:56Z

final_task/tests/unit/lexer/test_lexer.py

+
+    @unittest.skip('not implemented')
+    def test_peek(self):
+        pass


not implemented

AlexeiBuzuma · 2019-06-12T13:48:47Z

final_task/pycalc/calculator/calculator.py

+        raise CalculatorCalculationError(err_msg)
+
+
+def calculator(modules_names=None):


Нэйминг немного запутывает. В данном модуле есть класс калькулятор и функция калькулятор

AlexeiBuzuma · 2019-06-12T13:56:11Z

В основном код достаточно чистый и легко читаемый со стилистической точки зрения, однако я вижу тут одну комплексную проблему.
Во многих местах есть лишний код. Например есть большое количество функций/методов, которые просто проксируют вызов в другие функции.
Есть исключения, которые используются только в одном месте (можно было просто сделать более общее исключение для группы исключений и передавать разный текст ошибки.) Есть большое количество переменных, создание которых можно было легко избежать.

Код ощущается перенагруженным, большая его часть можно сказать ничего не делает.

siarhiejkresik added 30 commits April 11, 2019 14:51

Add setup.py to the project

79b6f93

Add Makefile

1fb4f8a

Change the program name in the setup.py

e1440e9

setup.py

ccf6979

Makefile

58f0d9a

Makefile

355ad34

Args

73f6f1f

Implement importing members from a module

248bb2c

Update importing from a module

f9adf74

Change members names storing from set to list

4573b75

Refactor, fix a bug, add exception handling

ecc888b

Add a function checker

c7a16ce

Format code

9f9ec2f

Rename imports.py file to importer.py

0a12d15

Add lexer

d05df4d

Add helpers functions for the matcher package

976bfa3

Add the numbers matcher

eae4290

Add the Matchers class

126e16e

Add __init__.py for the matcher package

27cab52

Add setup.py to the project

0963805

Add Makefile

a953f28

Refactor lexer

26e5dd8

Refactor lexer

8462382

Add parser

7f72e35

Update lexer

865675d

Add lexer tests

c22a61b

Refactor lexer

efe18dd

Move code blocks in the parser class

42b4399

Refactor lexer with comsumer/peek

4a9b556

Fix tests for lexer

c93d2ed

siarhiejkresik and others added 12 commits June 5, 2019 16:00

Add test running into the travis config

1d19055

Refactor lexer tests and move them into a proper directory

1fd5bca

Wrap parsing of command line arguments in a function

0ffe2a8

Add exceptions for calculator errors

883f4e7

Refactor calculator’s formatters

11d0b01

Fix imports

f29d325

Refactor formatters of calculator

76974cd

Raise exceptions on calculator errors

9231382

Refactor cli to class

31f1f61

Fix the top-level main script

5ee52e8

Remove unrelevant tests after refactoring

2d3276a

Add README file

43f8e3b

siarhiejkresik changed the title ~~[WIP] Siarhiej Kresik~~ Siarhiej Kresik Jun 5, 2019

PythonAndGoCourses2 added the Code Freeze #2 label Jun 6, 2019

AlexeiBuzuma reviewed Jun 12, 2019

View reviewed changes

final_task/tests/unit/lexer/test_lexer.py

@unittest.skip('not implemented')

def test_peek(self):

pass

Copy link

Collaborator

AlexeiBuzuma Jun 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not implemented

AlexeiBuzuma reviewed Jun 12, 2019

View reviewed changes

zaja1kun added the Reviewed label Jun 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Siarhiej Kresik #32

Siarhiej Kresik #32

siarhiejkresik commented May 10, 2019 •

edited

Loading

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

siarhiejkresik Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma Jun 12, 2019

AlexeiBuzuma commented Jun 12, 2019

		return sorted(iterable, key=len, reverse=True)


		def construct_literals_list(literals):

		raise CalculatorCalculationError(err_msg)


		def calculator(modules_names=None):

Siarhiej Kresik #32

Are you sure you want to change the base?

Siarhiej Kresik #32

Conversation

siarhiejkresik commented May 10, 2019 • edited Loading

Description

Minimal requirements

Mathematical operations calculator must support

Non-functional requirements

Distribution

Codestyle

Optional requirements

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexeiBuzuma commented Jun 12, 2019

siarhiejkresik commented May 10, 2019 •

edited

Loading