Гость
Целевая тема:
Создать новую тему:
Автор:
Форумы / Java [игнор отключен] [закрыт для гостей] / Код ревью тестового задания Яндекс / 10 сообщений из 10, страница 1 из 1
24.08.2014, 16:17
    #38727532
AlexeyKh24
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
Всем добрый вечер. Недавно делал тестовое задание Яндекса на написание 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.
package rectangular;

import static org.junit.Assert.*;
import org.junit.*;

public class TestRtriangleProvider {
	
	public static Rtriangle rtriangle;
	public static double x1, x2, x3, y1, y2, y3;
	
	@BeforeClass
	public static void setUp() {
	
		rtriangle = RtriangleProvider.getRtriangle(); // Получаем прямоугольный треугольник 
		assertNotNull("Rtriangle is null!", rtriangle); 
		x1 = rtriangle.getApexX1();  // Получаем координаты вершин
		x2 = rtriangle.getApexX2();
		x3 = rtriangle.getApexX3();
		y1 = rtriangle.getApexY1();
		y2 = rtriangle.getApexY2();
		y3 = rtriangle.getApexY3();
	}
	
	@Test
	public void testGetRtriangle () {
			
		double hypotenuse, cathetus1, cathetus2; // Гипотенуза, 2 катета - их необходимо определить
		double side1, side2, side3; // Стороны треугольника
		side1 = getLenght(x1, y1, x2, y2);
		side2 = getLenght(x1, y1, x3, y3);
		side3 = getLenght(x3, y3, x2, y2);
		assertTrue(side1!=0 & side2!=0 & side3!=0); // Проверяем, что нет сторон равных 0.
		
		if (side1 > side2) {  // Определяем гипотенузу и катеты
			if (side1 > side3) {
				hypotenuse = side1;
				cathetus1 = side2;
				cathetus2 = side3;
			} else {
				hypotenuse = side3;
				cathetus1 = side2;
				cathetus2 = side1;
			}
		} else {
			if (side2 > side3) {
				hypotenuse = side2;
				cathetus1 = side1;
				cathetus2 = side3;
			} else {
				hypotenuse = side3;
				cathetus1 = side2;
				cathetus2 = side1;
			}
		}
		
		System.out.println(side1 + " " + side2 + " " + side3);
		System.out.println(x1 + " " + y1 + "  " + x2 + " " + y2 + "  " + x3 + " " + y3);
		assertEquals(Math.pow(hypotenuse, 2.0), Math.pow(cathetus1, 2.0) + Math.pow(cathetus2, 2.0), 0.00001); // Проверяем по теореме Пифагора
	}
	
	public double getLenght(double x1, double y1, double x2, double y2) { // Метод вычисления длины одной из сторон треугольника
		double lenght = 0;
		lenght = Math.sqrt(Math.pow((x2-x1),2.0) + Math.pow((y2-y1),2.0));
		return lenght;
	}
}
...
Рейтинг: 0 / 0
24.08.2014, 17:12
    #38727555
just_vladimir
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
Года идут, задания не меняются :-) http://www.sql.ru/forum/918088/testovoe-zadanie
...
Рейтинг: 0 / 0
24.08.2014, 17:23
    #38727558
AlexeyKh24
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
just_vladimirГода идут, задания не меняются :-) http://www.sql.ru/forum/918088/testovoe-zadanie

Это да, но в той теме особо ничего интересного не нашел. Особенно увидев нижеприведенный там код - хочется "убивать" =)

Код: java
1.
2.
3.
4.
5.
6.
public int getApexX1() { return 5; }
			public int getApexY1() { return 5; }
			public int getApexX2() { return 5; }
			public int getApexY2() { return 5; }
		        public int getApexX3() { return 5; }
			public int getApexY3() { return 5; }



Просто хочу понять, что такого ужасного в моем коде, что меня даже не пригласили на собеседоваие? Поиск гипотенузы; то что я в реализации класса сделал поля double, а не int; что-то еще?
...
Рейтинг: 0 / 0
25.08.2014, 00:26
    #38727717
Westtrd
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
AlexeyKh24,

авторНапишите код Junit-тест, который будет проверять, действительно ли метод getRtriangle возвращает прямоугольный треугольник.

В этом причина
...
Рейтинг: 0 / 0
28.08.2014, 12:43
    #38731471
chebaaagh
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
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 в коде и тесте
...
Рейтинг: 0 / 0
28.08.2014, 12:46
    #38731480
chebaaagh
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
AlexeyKh24, название метода не должно быть testGetRtriangle(), обычно их называют по-другому: http://stackoverflow.com/questions/96297/what-are-some-popular-naming-conventions-for-unit-tests
...
Рейтинг: 0 / 0
28.08.2014, 13:03
    #38731501
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
AlexeyKh24Каюсь конечно, что с 34 по 54 - быдлокодерство
Возможно, оно даже было решающим, чтобы не пройти тест.

AlexeyKh24т.к. все это можно было положить в TreeSet, например, но с другой стороны так делать нельзя, т.к. катеты могут быть равными. Жду пинков, советов и т.п. :-)
И что если катеты будут равны? Можно было хоть через Math.max() искать. Но if...else здесь худший вариант.
...
Рейтинг: 0 / 0
28.08.2014, 13:04
    #38731504
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
lenght <> length
идея бы подстветила. эклипс не умеет разве?
...
Рейтинг: 0 / 0
28.08.2014, 13:34
    #38731556
ivanra
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
Если придраться, то вычислять корни тут тоже ни к чему.
Более того, если бы шла речь о производительности, то и в степень возводить не надо, достаточно вычислить 3 величины
Код: plaintext
1.
2.
3.
z 1  = x 2 x 3  + y 2 y 3 
z 2  = x 1 x 3  + y 1 y 3 
z 3  = x 1 x 2  + y 1 y 2 
и проверить, что какие-то 2 из них дают в сумме третью.
Хотя, конечно, это тесты, и можно плевать на производительность
...
Рейтинг: 0 / 0
29.08.2014, 09:49
    #38732373
ivanra
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Код ревью тестового задания Яндекс
ivanra
Код: plaintext
1.
2.
3.
z 1  = x 2 x 3  + y 2 y 3 
z 2  = x 1 x 3  + y 1 y 3 
z 3  = x 1 x 2  + y 1 y 2 

насчет этого уравнения наврал, величины другие, хотя тоже находятся за 6 умножений. Тем не менее, задача должна решаться строгим равенством, так как на входе целые числа, а при умножении можно перейти на long.
Что касается ошибок в самом оформлении теста, то к перечисленному можно добавить
- использование статических переменных
- соответственно, @BeforeClass вместо @Before, да и вообще это тут ни к чему
- если RtriangleProvider.getRtriangle() возращает не константу, то желательно в тесте организовать цикл и проверить метод N раз
...
Рейтинг: 0 / 0
Форумы / Java [игнор отключен] [закрыт для гостей] / Код ревью тестового задания Яндекс / 10 сообщений из 10, страница 1 из 1
Найденые пользователи ...
Разблокировать пользователей ...
Читали форум (0):
Пользователи онлайн (0):
x
x
Закрыть


Просмотр
0 / 0
Close
Debug Console [Select Text]