Гость
Войти | Профиль | Очистить
Целевая тема:
Создать новую тему:
Автор:
Действия ...
Форумы / Java [игнор отключен] [закрыт для гостей] / Ревью кода / 7 сообщений из 7, страница 1 из 1
03.09.2014, 14:44
    #38736258
Psolao
Psolao
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Пожалуйста взгляните на код, не перемудрил ли я с Женериками. Код заполняет Враппер для иерархического контейнера бинов. Код работает как нужно.
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
import java.util.Collection;

public interface Hierarchical<T extends Hierarchical> {

	public T getParent();
	public void setParent(T parent);	
	public Collection<T> getChildren();
	
}



Код: 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.
import java.io.Serializable;
import java.util.Collection;
import java.util.List;

import org.hibernate.Query;
import org.hibernate.Session;

import com.vaadin.data.util.BeanItemContainer;
import com.vaadin.data.util.ContainerHierarchicalWrapper;

	public static List getList(Session hbSession, String queryName) {
		hbSession.beginTransaction();
		Query query = hbSession.getNamedQuery(queryName);
		List list = query.list();
		hbSession.getTransaction().commit();
		return list;
	}

	public static <T extends Hierarchical<T>> ContainerHierarchicalWrapper getContainerHierarchicalWrapper(
			Session hbSession, String rootQuery, Class<T> BeanType) {
		BeanItemContainer<T> container = new BeanItemContainer(BeanType);
		ContainerHierarchicalWrapper beansWrapper = new ContainerHierarchicalWrapper(
				container);
		List<T> list = BaseDAO.getList(hbSession, rootQuery);
		{
			for (T item : list) {
				container.addBean(item);
				addChild(item, container);
			}
		}
		beansWrapper.updateHierarchicalWrapper();
		for (T obj : (Collection<T>) (beansWrapper.getItemIds()))
			beansWrapper.setChildrenAllowed(obj, false);
		for (T obj : (Collection<T>) (beansWrapper.getItemIds())) {
			T item = obj;
			T parent = item.getParent();
			beansWrapper.setChildrenAllowed(parent, true);
			beansWrapper.setParent(item, parent);
		}
		return beansWrapper;
	}

	private static <T extends Hierarchical<T>> void addChild(T item,
			BeanItemContainer<T> container) {
		if (item.getChildren() == null || item.getChildren().isEmpty())
			return;
		for (T child : (Collection<T>) (item.getChildren())) {
			container.addBean(child);
			addChild(child, container);
		}
	}

}
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 15:02
    #38736286
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Psolaoне перемудрил ли я с Женериками
"с генериками"
Psolao
Код: java
1.
2.
3.
4.
5.
6.
7.
public interface Hierarchical<T extends Hierarchical> {
        //Классика жанра. :)
	public T getParent();
	public void setParent(T parent);	
	public Collection<T> getChildren();
        //А такая иерархия разрешает использовать наследников?
}



Код: 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.
        //Тут бы, как раз, добавить Generic в List и @SuppressWarning
	public static List getList(Session hbSession, String queryName) {
		hbSession.beginTransaction();
		Query query = hbSession.getNamedQuery(queryName);
		List list = query.list();
		hbSession.getTransaction().commit();
		return list;
	}
        
        //ContainerHierarchicalWrapper - очень общее имя для типа. HierarchicalContainerWrapper не было бы более правильным с точки зрения английского языка?
	public static <T extends Hierarchical<T>> ContainerHierarchicalWrapper getContainerHierarchicalWrapper(
			Session hbSession, String rootQuery, 
                       Class<T> BeanType) { //Не правильный case идентифиактора. Должно быть beanType
		BeanItemContainer<T> container = new BeanItemContainer(BeanType);
		ContainerHierarchicalWrapper beansWrapper = new ContainerHierarchicalWrapper(
				container);
                //@SuppressWarning("unchecked")
		List<T> list = BaseDAO.getList(hbSession, rootQuery);
		{ //Что за скобки?
			for (T item : list) {
				container.addBean(item);
				addChild(item, container);
			}
		}
		beansWrapper.updateHierarchicalWrapper();

                //Здесь обязательно нужны фигурные скобки. Нельзя тело переносить на новую строку без них.
		for (T obj : (Collection<T>) (beansWrapper.getItemIds()))
			beansWrapper.setChildrenAllowed(obj, false);
                //Эти два цикла - полный WTF. Почему два цикла, а не один? ПОчему сначал всем запрещаются дети, потом тут же разрешаются? Понять что этот код делает - решительно не возможно.
		for (T obj : (Collection<T>) (beansWrapper.getItemIds())) {
			T item = obj;
			T parent = item.getParent();
			beansWrapper.setChildrenAllowed(parent, true)
			beansWrapper.setParent(item, parent);
		}
		return beansWrapper;
	}

	private static <T extends Hierarchical<T>> void addChild(T item,
			BeanItemContainer<T> container) {
                //Вызывается один и тот же метод getChildren() - три раза. Нужен introduce variable рефакторинг
		if (item.getChildren() == null || item.getChildren().isEmpty())
			return; //Снова потерялись скобки. Переносить return на новую строку здесь нет никакого сымсла
                //Зачем тут cast? Цикл - полный копипаст аналогично цикла выше по коду.
		for (T child : (Collection<T>) (item.getChildren())) {                        
			container.addBean(child);
			addChild(child, container);
		}
	}

}
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 15:45
    #38736418
Psolao
Psolao
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Blazkowicz, большое спасибо. Пару вещей оставил как есть, поскольку там не мои компоненты, например ContainerHierarchicalWrapper, я его не переименую никак ). Отдельное спасибо за совет со скобками, при переносе на другую строку. А код
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
		for (T obj : (Collection<T>) (beansWrapper.getItemIds()))
			beansWrapper.setChildrenAllowed(obj, false);
		for (T obj : (Collection<T>) (beansWrapper.getItemIds())) {
			T item = obj;
			T parent = item.getParent();
			beansWrapper.setChildrenAllowed(parent, true);
			beansWrapper.setParent(item, parent);
		}


не совсем верный, хотя и работал правильно, я исправил
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
		// изначально выставляем, что детей ни у кого нет
		for (T obj : (Collection<T>) (beansWrapper.getItemIds())){
			beansWrapper.setChildrenAllowed(obj, false);
		}	
		// пробегаем по всем элементам, если у элемента родитель существует,
 		// связываем его с ребенком, и выставляем родителю признак
		// того, что он имеет детей
		for (T obj : (Collection<T>) (beansWrapper.getItemIds())) {
			T item = obj;
			T parent = item.getParent();
			if (parent != null) {
			  beansWrapper.setChildrenAllowed(parent, true);
			  beansWrapper.setParent(item, parent);
			}  
		}
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 15:54
    #38736441
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Забыл написать
beansWrapper - правильно beanWrapper. Даже, если там много бинов, всё равно первое слово должно быть в ед. числе.
И всё равно не понятно, почему два цикла, а не один с if...else?
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 16:03
    #38736465
забыл ник
забыл ник
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Да и без if\else можно обойтись думаю, по умолчанию то наверняка false
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 16:08
    #38736479
Psolao
Psolao
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
Blazkowicz, а все равно два цикла будет, как не крути, только тут они вложенные. В исходном коде я выставляю признак наличия детей не перебираемому элементу, а его родителю. Те элементы которые мне не встретятся в цикле как родители должны иметь значение false.

Вот альтернативный вариант.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
		for (T obj : (Collection<T>) (beanWrapper.getItemIds())) {
			T item = obj;
			Collection<T> children = item.getChildren();
			if (children != null && !children.isEmpty()) {
			  beanWrapper.setChildrenAllowed(item, true);
			  for (T child:children){
				  beanWrapper.setParent(child, item);  
			  }			  
			}  
			else beanWrapper.setChildrenAllowed(item, false);
		 }
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
03.09.2014, 16:15
    #38736492
Psolao
Psolao
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Ревью кода
забыл ник, По умолчанию там true :)
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
Форумы / Java [игнор отключен] [закрыт для гостей] / Ревью кода / 7 сообщений из 7, страница 1 из 1
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение  
Полит. конфиден.  
созд. / загр.: 450ms
Закрыть
start [/forum/topic.php?fid=59&tablet=1&tid=2126654]:
 0ms
get settings:
 7ms
get forum list:
 20ms
check forum access:
 3ms
check topic access:
 3ms
track hit:
 148ms
get topic data:
 10ms
get forum data:
 2ms
get page messages:
 50ms
get tp. blocked users:
 1ms
others: 206ms
total:  450ms
x
x
Закрыть


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