|
|
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
Всем добрый вечер. Недавно делал тестовое задание Яндекса на написание jUnit теста для проверки метода, который генерирует прямоугольные треугольники. Подробнее задача описана тут http://company.yandex.ru/job/vacancies/test_automation_engineer.xml?ncrnd=8285 - наверное многие уже ее видели. Видимо им не понравилось мое "произведение искусства", т.к. на собеседование меня не пригласили. Сам Яндекс скорее всего код ревью мне не предоставит, решил провести его здесь. Жду советов. Каюсь конечно, что с 34 по 54 - быдлокодерство, т.к. все это можно было положить в TreeSet, например, но с другой стороны так делать нельзя, т.к. катеты могут быть равными. Жду пинков, советов и т.п. :-) Полный проект здесь https://bitbucket.org/preda/yandexrtriangle.git Код: java 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. 17. 18. 19. 20. 21. 22. 23. 24. 25. 26. 27. 28. 29. 30. 31. 32. 33. 34. 35. 36. 37. 38. 39. 40. 41. 42. 43. 44. 45. 46. 47. 48. 49. 50. 51. 52. 53. 54. 55. 56. 57. 58. 59. 60. 61. 62. 63. 64. 65. 66. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 24.08.2014, 16:17 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
Года идут, задания не меняются :-) http://www.sql.ru/forum/918088/testovoe-zadanie ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 24.08.2014, 17:12 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
just_vladimirГода идут, задания не меняются :-) http://www.sql.ru/forum/918088/testovoe-zadanie Это да, но в той теме особо ничего интересного не нашел. Особенно увидев нижеприведенный там код - хочется "убивать" =) Код: java 1. 2. 3. 4. 5. 6. Просто хочу понять, что такого ужасного в моем коде, что меня даже не пригласили на собеседоваие? Поиск гипотенузы; то что я в реализации класса сделал поля double, а не int; что-то еще? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 24.08.2014, 17:23 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
AlexeyKh24, авторНапишите код Junit-тест, который будет проверять, действительно ли метод getRtriangle возвращает прямоугольный треугольник. В этом причина ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 25.08.2014, 00:26 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
Westtrd, все ниже изложенное сугубо личное 1. package назван не по конвенции 2. класс назван глаголом 3. поля x1, x2, x3, y1, y2, y3, которые здесь не нужны 4. область видимости методов и полей должна быть минимальной 5. комментарии по-русски 6. по поводу кода с определением гипотенузы и катетов: может стоило подумать, как избежать этого, например найти только минимальную и среднуюю по длине стороны, и посчитать скалярное произведение 7. поле rtriangle не должно быть статическим 8. вывод инфы в консоль можно было и убрать, т.к. он не несет какой-либо полезной информации 9. double lenght = 0; lenght = Math.sqrt(Math.pow((x2-x1),2.0) + Math.pow((y2-y1),2.0)); return lenght; зачем здесь переменная length? 10. 0.00001 можно вынести в private static final double EQUALITY_PRECISION, например 11. возможно, стоило использовать BigDecimal в коде и тесте ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 28.08.2014, 12:43 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
AlexeyKh24, название метода не должно быть testGetRtriangle(), обычно их называют по-другому: http://stackoverflow.com/questions/96297/what-are-some-popular-naming-conventions-for-unit-tests ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 28.08.2014, 12:46 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
AlexeyKh24Каюсь конечно, что с 34 по 54 - быдлокодерство Возможно, оно даже было решающим, чтобы не пройти тест. AlexeyKh24т.к. все это можно было положить в TreeSet, например, но с другой стороны так делать нельзя, т.к. катеты могут быть равными. Жду пинков, советов и т.п. :-) И что если катеты будут равны? Можно было хоть через Math.max() искать. Но if...else здесь худший вариант. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 28.08.2014, 13:03 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
lenght <> length идея бы подстветила. эклипс не умеет разве? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 28.08.2014, 13:04 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
Если придраться, то вычислять корни тут тоже ни к чему. Более того, если бы шла речь о производительности, то и в степень возводить не надо, достаточно вычислить 3 величины Код: plaintext 1. 2. 3. Хотя, конечно, это тесты, и можно плевать на производительность ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 28.08.2014, 13:34 |
|
||
|
Код ревью тестового задания Яндекс
|
|||
|---|---|---|---|
|
#18+
ivanra Код: plaintext 1. 2. 3. насчет этого уравнения наврал, величины другие, хотя тоже находятся за 6 умножений. Тем не менее, задача должна решаться строгим равенством, так как на входе целые числа, а при умножении можно перейти на long. Что касается ошибок в самом оформлении теста, то к перечисленному можно добавить - использование статических переменных - соответственно, @BeforeClass вместо @Before, да и вообще это тут ни к чему - если RtriangleProvider.getRtriangle() возращает не константу, то желательно в тесте организовать цикл и проверить метод N раз ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 29.08.2014, 09:49 |
|
||
|
|

start [/forum/topic.php?fid=59&msg=38732373&tid=2126677]: |
0ms |
get settings: |
9ms |
get forum list: |
16ms |
check forum access: |
4ms |
check topic access: |
4ms |
track hit: |
177ms |
get topic data: |
9ms |
get forum data: |
4ms |
get page messages: |
42ms |
get tp. blocked users: |
1ms |
| others: | 231ms |
| total: | 497ms |

| 0 / 0 |
