Skip to content

Conversation

timonov
Copy link
Contributor

@timonov timonov commented Oct 25, 2015

No description provided.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This method is not covered by tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test added.

@okainov
Copy link
Collaborator

okainov commented Oct 26, 2015

И я бы не стал делать метод вычисления определителя (и некоторые другие) "статическим", логично, что он должен относится к объекту. Ну и визуально даже очевидно, что проще читается:

print Matrix.calculate_det(my_matrix)

Или

print my_matrix.calculate_det()

@timonov
Copy link
Contributor Author

timonov commented Oct 28, 2015

Updated method of determinant calculation too.

@timonov
Copy link
Contributor Author

timonov commented Oct 28, 2015

@AlekseyNesmelov
@AnastasiaGarina
@GodfatherThe
@KsenyaKochanova
@ZhbanovaAnna
Guys, please review.

@okainov
Copy link
Collaborator

okainov commented Oct 29, 2015

Где же тест на бросание исключения?

Choose a reason for hiding this comment

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

Первый тест test_check_constructor мне кажется немного некорректным. Думаю, имеет смысл разделить его на 2 теста, отдельно проверять, что создается матрица нужного размера и отдельно проверить значения. Условия из if записать в assertTrue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Тест должен проверять корректное создание обьекта. Один тест на один обьект. А если у обьекта не три поля, а больше, сколько тогда делать тестов? Для каждого поля по тесту?

Choose a reason for hiding this comment

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

по сути у тебя уже написаны проверки для каждого поля, только не в assert, а в if. я не настаиваю на том, что мой вариант идеален, возможно здесь что-то получше можно придумать, но так как есть точно оставлять не вариант. Смотри, если у тебя в if условие вдруг не выполниться, тест все равно не будет красным, то есть свою задачу обнаружения ошибок он не выполняет, вот в этом его некорректность, а не в том, что он один, а не два. признаю, я не очень ясно выразилась в прошлом комменте)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Исправлено

@timonov
Copy link
Contributor Author

timonov commented Nov 9, 2015

@Daniil-Osokin
@kirill-kornyakov

AoD314 added a commit that referenced this pull request Nov 9, 2015
Тимонова - Лабораторная работа #1
@AoD314 AoD314 merged commit a564ee5 into UNN-ITMM-Software:master Nov 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants