-
Notifications
You must be signed in to change notification settings - Fork 55
hw_zolotikov #7
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: main
Are you sure you want to change the base?
hw_zolotikov #7
Conversation
uzolotikov
commented
Sep 14, 2023
- Золотиков
- Волкова
- Огурцова
- Федоренко
- Козин
Add div(),main() to calculator.py
Add function sum
Add multiply and substruction functions
nvaulin
left a comment
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.
Хорошая работа!
Оставил в коде некоторые замечания по оформлению. Они не критические, просто обратите на них внимание на будущее. Еще раз обратите внимание на то как принято делать сообщения коммитов. Вот эти названия у вас образцовые:
Баллы:
К сожалению у вас тимлид в итоге член команды который не добавил никакую функцию, хотя в задании написано 1.6 балла за каждую пару автор-функция (ситуация с Ваней не в счёт), всё таки задание и по гиту и по питону. Плюс в README не хватает описания вашей мини-программы (есть только состав команды): - 0.4 балла.
- За функции: 1.6 * 4 = 6.4
- За README 0.6 + 1 доп. = 1.6
- За наличие форков и пулл-реквестов - 1 балл
Итого: 9 баллов
Мерджить не надо, но стоит скинуть показать комментарии всем участникам команды
Но в целом все сделали правильно.
Молодцы!
| @@ -0,0 +1,27 @@ | |||
| def mymult(x, y): | |||
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.
Если так называть наверное чуть лучше было бы my_mult. Хотя не знаю.
| @@ -0,0 +1,27 @@ | |||
| def mymult(x, y): | |||
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.
Здесь и далее лучше использовать более информативные названия переменных. Названия x и y не отражают содержимого. Лучше было бы, например, num1 и num2. Здесь за это баллы еще не снижаем, но с ДЗ 4 нейминг начнет влиять на оценку.
| def mysub(x, y): | ||
| return x - y | ||
| def mysum(a, b): | ||
| return a+b |
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.
Не хватает пробелов
| return a+b | |
| return a + b |
| return x/y | ||
| def main(): |
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.
Между определениями функций принято оставлять 2 пустые строки (ссылка). Тут это не критично, но с ДЗ 4 может влиять на оценку.
| return x/y | |
| def main(): | |
| return x/y | |
| def main(): |
| def mydiv(x,y): | ||
| return x/y | ||
| def main(): | ||
| inp = input('vvedite to, chto nado poschitat') |
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.
- 'vvedite to, chto nado poschitat' - вот это не очень хорошо))
По-русски или по-английски. Понимаю что шутка, но все таки не мог не заметить. inp.split(sep =' ')не обязательно было указывать сепаратор, он такой по умолчанию, но ок- inp в данном случае более-менее понятное, но все таки не очень супер имя переменной.
| inp = input('vvedite to, chto nado poschitat') | ||
| inp = inp.split(sep =' ') | ||
| x = float(inp[0]) | ||
| y = float(inp[2]) | ||
| sign = inp[1] |
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.
В целом код рабочий, хоть и не очень питонячий. Это ничего страшного, мы с вами еще научимся. Например, тут это можно было бы сделать так:
| inp = input('vvedite to, chto nado poschitat') | |
| inp = inp.split(sep =' ') | |
| x = float(inp[0]) | |
| y = float(inp[2]) | |
| sign = inp[1] | |
| num1, operator, num2 = input().split() | |
| num1, num2 = float(num1), float(num2) |
Название sign в целом не плохое, но я не уверен что в английском "знак" в смысле математического знака так и переводится. Поэтому я бы сделал operator, хоть так и подлиннее.
| else: | ||
| result = "Oshibka" |
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.
Хорошо что сделали else который обрабатывает любой другой неподдерживаемый оператор. Обработать такую ситуацию можно было бы на самом деле чуть аккуратнее, но этому мы еще научимся.
| result = mydiv(x,y) | ||
| else: | ||
| result = "Oshibka" | ||
| print(result) |
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.
Все-таки наверное лучше было бы сделать через return, но наверное это моя неточность в ТЗ.
| else: | ||
| result = "Oshibka" | ||
| print(result) | ||
| main() No newline at end of file |
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.
Такие вещи еще зачастую пишут через
if __name__ == '__main__':
main()На следующей лекции как раз разберем зачем это нужно
| @@ -0,0 +1,7 @@ | |||
| **Состав команды**: | |||
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.
Круто что вы перечислили обязанности каждого члена команды 👍
В будущем будет здорово если будете тренироваться делать README на английском