ReSQL.ru
     
powered by simpleCommunicator - 2.0.61     © 2026 Programmizd 02
Мобильная версия    Контакт    Правила    FAQ    Помощь
Целевая тема:
Создать новую тему:
Автор:
Закрыть
Цитировать
Доб. в избранное | Игнор. тему | Прикреп. тему | Пометить прочит. / непрочит. | Фильтр
Форумы / Java [игнор отключен] [закрыт для гостей] / Покритикуйте код. Заход 2
25 сообщений из 60, страница 2 из 3
  2  все
Покритикуйте код. Заход 2
    #38525872
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
GaraZа как лучше сделать? не испотльзовать один класс дя доступа к другим а передавать их все в конструктор (Допустим MainForm)

Inversion of Control -> Dependency Injection


GaraZКогда из Дерева удаляем ноду - фокус должен переместится на следущую или на первую - если удаленная нода последняя

В JTree есть методы работы со строками (rows). Реально какой-то сложный метод, который, ИМХО, можно было бы заменить методами из JTree.

GaraZBlazkowiczline.separator разве работает в JOptionPane? По-моему нужно JTextArea использовать вместо строки

а почему нет

По-тому что jlabel не умеет показывать многострочный текст, без <html>.
...
Рейтинг: 0 / 0
14.01.2014, 16:22
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525891
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
Blazkowicz,

BlazkowiczreadFile не закрывает стримы. Почему try-with-resource не используется?

провтыкал (

BlazkowiczSiteHelper
- длинный список аргументов я предпочитаю форматировать в столбик.
- GetThread - ни о чем. Что за поток? Чем занимается?
- GetThread - цикла в run не вижу. Зачем надо было наследоваться от Thread? Используйте Executors/SwingWorker.
- Есть потоки, но обращение к GUI не выполнено из EDT. Срочно читать статьи о потоках в Swing.
- parse методы - каша, вникать лень.

Поток парсит страницу выдирая с нее картинки
Для каждой страницы в runSitesDownload(CustomList<SiteObj> sitesList)
создается свой поток GetThread
Цикл в run и не нужен


Blazkowiczclass Timer implements Runnable
Really? То есть java.util.Timer, javax.swing.Timer и ScheduledExecutorService на столько кривые, что пришлось написать свой?

Ничем мой класс не лучше. Просто мне нужно было и jLabelTime (Циферблат) менять каждую секунду и еще и задание выполнить через n секунд
После беглого просмотра по существующим классам понял что для этого придется применить два таймера
Ну может это и не так. Чесно говоря сильно не вникал.

BlazkowiczПо-тому что jlabel не умеет показывать многострочный текст, без <html>.

Ну незнаю )0 у меня все переносит
...
Рейтинг: 0 / 0
14.01.2014, 16:37
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525892
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
MasterZivBlazkowicz,

а оно работает ? Использовать можно по назначению?

Если вас интересует работает ли приложение - то да вполне
я каждый день его юзаю для своей группы
Мне интересно было бы если бы ктото еще попытался его заюзать )))
...
Рейтинг: 0 / 0
14.01.2014, 16:40
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525895
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
GaraZПоток парсит страницу выдирая с нее картинки
Для каждой страницы в runSitesDownload(CustomList<SiteObj> sitesList)
создается свой поток GetThread
Цикл в run и не нужен

Если не нужен цикл в run, то и Thread не нужен.
Нужны Executors и Callable/Runnable.
Если нужна интеграция с UI, то SwingWorker.
Зачем тут Thread не понятно.

GaraZНу незнаю )0 у меня все переносит
Значит я гоню. Переносы работают. Это word wrap не работает.
...
Рейтинг: 0 / 0
14.01.2014, 16:41
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525980
Фотография mayton
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
mayton
Участник
BlazkowiczMasterZivBlazkowicz,

а оно работает ? Использовать можно по назначению?
Это к какому моему коментарию вопрос?
Не спорьте по пустякам. Это я опечатался. Задачу получения HOME каталога можно решать
на разных уровнях. Можно проверить версию ОС и использовать тильду или %USER_PROFILE%
...
Рейтинг: 0 / 0
14.01.2014, 17:34
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525984
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
maytonBlazkowiczMasterZivBlazkowicz,
а оно работает ? Использовать можно по назначению?
Это к какому моему коментарию вопрос?
Не спорьте по пустякам. Это я опечатался.
Кто здесь?
...
Рейтинг: 0 / 0
14.01.2014, 17:36
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525993
Basil A. Sidorov
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Basil A. Sidorov
Участник
maytonМожно проверить версию ОС и использовать тильду или %USER_PROFILE%Зачем, если есть стандартное свойство?
...
Рейтинг: 0 / 0
14.01.2014, 17:40
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38525997
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
Basil A. SidorovmaytonМожно проверить версию ОС и использовать тильду или %USER_PROFILE%Зачем, если есть стандартное свойство?
какое стандартное средство?
...
Рейтинг: 0 / 0
14.01.2014, 17:43
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526000
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
GaraZBasil A. Sidorovпропущено...
Зачем, если есть стандартное свойство?
какое стандартное средство?
свойство тоесть
...
Рейтинг: 0 / 0
14.01.2014, 17:44
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526005
ivanra
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
ivanra
Гость
BlazkowiczGregorianCalendar calendar = new GregorianCalendar(); лучше Calendar calendar = Calendar.newInstance();
Не знаю. Как-то привычнее и вопросов не вызывает. Почему именно GregorianCalendar... Абстракцию, ведь, для чего-то создали.
Можно поспорить насчет полезности данного совета (например, по одному из календарей сегодня новый год!!!)
на самом деле он там вообще не нужен, так как используется не по назначению (для получения текущего времени):
Код: java
1.
String dirNow = df.format(calendar.getInstance().getTime());
...
Рейтинг: 0 / 0
14.01.2014, 17:47
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526013
Basil A. Sidorov
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Basil A. Sidorov
Участник
GaraZGaraZкакое стандартное средство?свойство тоестьСмотрите вывод:
Код: sql
1.
jinfo PID|find "user"|more

если уж из топика непонятно.
...
Рейтинг: 0 / 0
14.01.2014, 17:50
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526023
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
Basil A. SidorovGaraZпропущено...
свойство тоестьСмотрите вывод:
Код: sql
1.
jinfo PID|find "user"|more

если уж из топика непонятно.
ааа
я подумал еще чето есть )
...
Рейтинг: 0 / 0
14.01.2014, 18:01
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526028
0FD
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
0FD
Участник
GaraZ,

SiteHelper :
private static Map<URI, byte[]> pageMap;
private static Map<URI, byte[]> imgMap;
private static List<Exception> exList;

Используются несколькими потоками GetThread
...
Рейтинг: 0 / 0
14.01.2014, 18:04
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526041
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
0FDGaraZ,

SiteHelper :
private static Map<URI, byte[]> pageMap;
private static Map<URI, byte[]> imgMap;
private static List<Exception> exList;

Используются несколькими потоками GetThread
ну да
что не так?
...
Рейтинг: 0 / 0
14.01.2014, 18:09
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526047
Лагман
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Лагман
Участник
GaraZ,

не являются thread safe
...
Рейтинг: 0 / 0
14.01.2014, 18:11
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526065
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
0FD, Лагман,
нежданчик
я даже и не подумал посмотреть являются ли они потоко-безопасными
явный прогон с моей стороны
спасибо )
...
Рейтинг: 0 / 0
14.01.2014, 18:18
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526129
Фотография mayton
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
mayton
Участник
GaraZBasil A. Sidorovпропущено...
Зачем, если есть стандартное свойство?
какое стандартное средство?
Я не возражаю. Но ему это стандартное свойство надо описать в терминах конфигурационного
файла. Предложите вариант как в строку вставить ссылку на USER_HOME
...
Рейтинг: 0 / 0
14.01.2014, 19:06
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526169
Basil A. Sidorov
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Basil A. Sidorov
Участник
maytonПредложите вариант как в строку вставить ссылку на USER_HOMEНичего не понял, честно говоря.
Общепринятая практика использует ${user.home}, а так же любое другое свойство установленное "самим запускатором" или заданное в его командной строке.
Код, который разбирает такие вставки, конечно, должен быть написан, но тот же log4j умеет это из коробки.
Можно, например, написать "обобщённый" server.xml и запускать какой-нибудь tomcat как:
Код: sql
1.
java -Dsite.port=XXXX -site.jvm=YYY -Dcatalina.base=ZZZ -jar bin/bootstrap.jar

Работает как из пушки.

P.S. Хотите извратиться - можно использовать "обрамление процентами", как в виндовом шелле или REXX-овый вариант - имена вне кавычек рассматриваются как переменная, а при наличии "правильно прижатых скобок" - как вызов функции.
...
Рейтинг: 0 / 0
14.01.2014, 19:44
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526171
Basil A. Sidorov
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Basil A. Sidorov
Участник
Basil A. Sidorov
Код: sql
1.
java -Dsite.port=XXXX -Dsite.jvm=YYY -Dcatalina.base=ZZZ -jar bin/bootstrap.jar
...
Рейтинг: 0 / 0
14.01.2014, 19:45
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526187
Фотография mayton
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
mayton
Участник
Нет. Нет. У чела есть конфиг. В нём прописано.

Код: xml
1.
2.
3.
4.
5.
6.
7.
<?xml version="1.0" encoding="UTF-8"?>

<Root>
  <Common>
    <GroupId></GroupId>
    <Comments></Comments>
    <ContentDir>/home/GaraZ/VKLoader/target/Cont</ContentDir>



Или

Код: xml
1.
  <ContentDir>c:\Documents And Settings\GaraZ\VKLoader\target\Cont</ContentDir>



Но суть - та-же самая. Каталог пользователя и локальный путь GaraZ\VKLoader\target\Cont
...
Рейтинг: 0 / 0
14.01.2014, 20:03
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38526199
Basil A. Sidorov
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Basil A. Sidorov
Участник
maytonНет. Нет. У чела есть конфиг. В нём прописаноПоскипано.
Должно быть:
Код: xml
1.
<ContentDir>${user.home}/VKLoader/target/Cont</ContentDir>

Код разбора или берётся готовый или пишется самостоятельно.

P.S. Для винды разворот слешей - вопрос личных предпочтений (файловому API - до лампочки).
...
Рейтинг: 0 / 0
14.01.2014, 20:14
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38539370
GaraZ
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
GaraZ
Участник
Постарался учесть изложенные выше замечания
Обновил репозиторий :)
...
Рейтинг: 0 / 0
27.01.2014, 16:58
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38539486
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
GaraZПостарался учесть изложенные выше замечания
Обновил репозиторий :)
App
http://docs.oracle.com/javase/tutorial/uiswing/concurrency/initial.html
initDependence() - почему не initDependencies(), почему не просто init()???
initialization это обычно этам подготовки к чему-то.
А здесь не только подготовка, но и сам запуск.
Ну, это такое...

ExceptionLoggerForm
Уже писал выше. Очень рекомендую всегда указывать родительский контейнер для JDialog и JFrame.

FileManager
initContent() - явно собирает какой-то список файлов по маске. Ну, почему было не дать методу внятное и понятное имя.
Иначе в таком стиле можно весь код написать
Код: java
1.
2.
3.
4.
public class GenericPurposeClass{
  private void init();
  private Result doThePurpose();
}


for (int i = 0; i < length; i++) - заменяется на for each.

Грубая ошибка:
Код: java
1.
2.
3.
4.
catch(NoSuchFileException e) {
            filesList.remove(file);
            throw new NoSuchFileException(file.getName().concat(" not found. "));
}


Вы теряете оригинальный stacktrace. Почему не просто throw e?
filesList.remove(file); копипастом вставлен в два места. Почему было не использовать try...finally без catch вообще?

synchronized стоит на методе класса FileManager, хотя по логике он относится к File.

saveImages/savePages - очень сильно смущает фривольное обращение с URI, который, вероятно, должен быть всегда валидным для этой задачи.


MainForm
Почему было import-ы не свернуть через * ?
getTreeCellRendererComponent - должен очень быстро по состоянию модели создать отображение. А тут запускается какая-то сложная логика с IO, ловятся исключения. А потом пишут мол Swing тормозной.

Download implements Runnable - запускается в фоновом потоке и из него обновляет GUI. Это приведёт к рандоммным исключениям с UI уровня Swing. Используйте SwingWorker или SecondaryLoop.

А это что за финт ушами?
Код: java
1.
2.
class Download implements Runnable {
        private Runnable runnable;



Как уже писал выше.
Лесенка if (evt.getSource() == jButtonFileDel) и смешивание UI с бизнес-логики в одном классе.
...
Рейтинг: 0 / 0
27.01.2014, 17:40
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38539505
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
Код: java
1.
2.
3.
4.
 @Override
    public String toString() {
        return login.trim();
    }


Лучше полностью выводить состояние объекта. Удобно для отладки. Не используйте toString() для логики.

SettingsForm модальный и не имеет родительского окна. Модальность сломана.
this перед полями не везде пишется. Github не достаточно умный, чтобы находить поля и подсвечивать.

Всё что касается сложной логики
SiteManager.parse() и методы VkManager - реальная каша. Читать трудно.
...
Рейтинг: 0 / 0
27.01.2014, 17:51
| Ответить | Цитировать | Написать  
Покритикуйте код. Заход 2
    #38539515
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
GaraZ,

Вам нужно научиться отделять модель от представления. Весь ваш код должен уметь работать без UI вообще.
UI нужен лишь для того чтобы отображать состояние вашей модели.
Хорошим упражнением может быть создание альтернативного UI для вашей логики. Например web или JavaFX.
...
Рейтинг: 0 / 0
27.01.2014, 17:54
| Ответить | Цитировать | Написать  
25 сообщений из 60, страница 2 из 3
  2  все
Форумы / Java [игнор отключен] [закрыт для гостей] / Покритикуйте код. Заход 2
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение   Полит. конфиден.  
NoSQL.ru       Кролег: Проекты, Новости, Чат       РЕД ФОРУМ       Фотоальбом: Участники, Встречи
Закрыть
start [/forum/topic.php?fid=59&msg=38539515&tid=2127739]:
 0ms
get settings:
 10ms
get forum list:
 15ms
check forum access:
 4ms
check topic access:
 4ms
track hit:
 253ms
get topic data:
 10ms
get forum data:
 2ms
get page messages:
 43ms
get tp. blocked users:
 1ms
others: 219ms
total:  561ms
созд. / загр.: 561ms
x
x
Закрыть


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