Модераторы: LSD, AntonSaburov
  

Поиск:

Ответ в темуСоздание новой темы Создание опроса
> В чем здесь быдлокодинг? Вообще, кто какие ошибки видит. 
:(
    Опции темы
apv1989
Дата 14.6.2011, 15:36 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Новичок



Профиль
Группа: Участник
Сообщений: 10
Регистрация: 22.5.2008

Репутация: нет
Всего: нет
Здравствуйте, учусь java (api в основном, язык то, конечно, си-стайл знаком), хотя в общем то программирую не первый год и это не первый мой язык ООП.
Да вот только когда старший программист увидел этот код, полез сначала на потолок, потом с него упал, потом закатился под стол, корчась толи от боли, то ли от смеха, не знаю в общем.
Вердикт был: зарефакторить абсолютно все. Каждую строчку буквально. 
Что тут такого страшного, скажите пожалуйста?

Код

public class ViewPhoto extends HttpServlet {
    private void sendPhoto(byte[] photo, HttpServletResponse resp) throws IOException {
        resp.setHeader("Cache-Control", "no-cache");
        resp.setHeader("pragma", "no-cache");
        String format = ImageIO.getImageReaders(ImageIO.createImageInputStream(new ByteArrayInputStream(photo))).next().getFormatName().toLowerCase();
        String mimeType = "image/" + format;
        resp.setContentType(mimeType);
        resp.setContentLength(photo.length);
        OutputStream out = resp.getOutputStream();
        out.write(photo);
        out.close();
    }

    @Override
    protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        int id = Integer.valueOf(req.getParameter("id"));
        String table = req.getParameter("writeTo");
        String type = req.getParameter("type");


        try {
            AddPetService service = BeanProvider.getBean();
            if ("mzsr".equals(table)) {
                MzsrAddressee addressee = service.getMzsrAddressee(id);
                if ("small".equals(type) && addressee.getBossSmallPhoto() != null) {
                    sendPhoto(addressee.getBossSmallPhoto(), resp);
                    return;
                } else if ("big".equals(type) && addressee.getBossPhoto() != null) {
                    sendPhoto(addressee.getBossPhoto(), resp);
                    return;
                }
            }
            if ("kiosk".equals(table)) {
                KioskAddressee addressee = service.getKioskAddressee(id);
                if ("small".equals(type) && addressee.getBossSmallPhoto() != null) {
                    sendPhoto(addressee.getBossSmallPhoto(), resp);
                    return;
                } else if ("big".equals(type) && addressee.getBossPhoto() != null) {
                    sendPhoto(addressee.getBossPhoto(), resp);
                    return;
                }
            }
        } catch (NamingException e) {
            resp.sendError(HttpServletResponse.SC_NOT_FOUND);
            return;
        }

        getServletContext().getRequestDispatcher("/i/no_photo.jpg").forward(req, resp);
    }
}
PM MAIL   Вверх
danilych
Дата 14.6.2011, 16:38 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Шустрый
*


Профиль
Группа: Участник
Сообщений: 72
Регистрация: 2.2.2011

Репутация: 3
Всего: 3
Цитата

A typical design mistake made by inexperienced developers is to mix different types of logic
(e.g., presentation logic, business logic, and data access logic) in a single large module. This
reduces the module’s reusability and maintainability...

из книги Spring Recipes A Problem-Solution Approach Gary Mak

В вашем коде смешана логика бизнес уровня, доступа к данным и презентационная.
Ваш код нереально тяжело будет поддерживать и развивать.
PM MAIL   Вверх
dobrolub
Дата 14.6.2011, 19:15 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Опытный
**


Профиль
Группа: Участник
Сообщений: 385
Регистрация: 18.12.2009
Где: Vancouver, Canada

Репутация: 11
Всего: 16
Я бы переписал логику более прямолинейно. (см. правильный коммент от самотника, внизу)

Код

    @Override
    protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        int id = Integer.valueOf(req.getParameter("id"));
        String table = req.getParameter("writeTo");
        String type = req.getParameter("type");
        try {
            AddPetService service = BeanProvider.getBean();
            AbstractAddressee addressee = null;
            if ("mzsr".equals(table))
              addressee = service.getMzsrAddressee(id);
            else if ("kiosk".equals(table))
              addressee = service.getKioskAddressee(id);
             
            byte[] photo = null;
            if (addressee == null) {
            }
            else if ("big".equals(type)) {
               photo = addressee.getBossPhoto();
            }
            else if ("small".equals(type)) {
               photo = addressee.getBossSmallPhoto();
            }
           
            if (photo != null)
               sendPhoto(photo, resp);
            else
               getServletContext().getRequestDispatcher("/i/no_photo.jpg").forward(req, resp);
        } catch (NamingException e) {
            e.printStackTrace();

            resp.sendError(HttpServletResponse.SC_NOT_FOUND);
        }
    }


Это сообщение отредактировал(а) dobrolub - 15.6.2011, 02:09
PM   Вверх
Samotnik
Дата 14.6.2011, 19:17 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Super star !
****


Профиль
Группа: Awaiting Authorisation
Сообщений: 7192
Регистрация: 4.11.2006
Где: Минск City

Репутация: 8
Всего: 191
apv1989, слишком много вложенных if/else конструкций и вызовов метода sendPhoto

Добавлено через 2 минуты и 38 секунд
dobrolub, даже решение выложил  smile 

Это сообщение отредактировал(а) Samotnik - 14.6.2011, 19:20
PM MAIL   Вверх
apv1989
Дата 15.6.2011, 09:13 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Новичок



Профиль
Группа: Участник
Сообщений: 10
Регистрация: 22.5.2008

Репутация: нет
Всего: нет
Спасибо всем.

Чувствую себя индусом.  smile 

C удовольствием бы AbstractAddressee сделал, но это две сущности JPA, абсолютно одинаковые, но привязанные к разным таблица, а mappedsuperclass использовать нельзя. 

Это сообщение отредактировал(а) apv1989 - 15.6.2011, 09:19
PM MAIL   Вверх
powerOn
Дата 15.6.2011, 12:27 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


software saboteur
****


Профиль
Группа: Участник
Сообщений: 4367
Регистрация: 7.10.2005

Репутация: 47
Всего: 159
я бы посмотрел в сторону Compose Method рефакторинга.


--------------------
user posted image нет времени думать - нужно писать КОД!
PM MAIL   Вверх
COVD
Дата 15.6.2011, 15:49 (ссылка) | (голосов:1) Загрузка ... Загрузка ... Быстрая цитата Цитата


Эксперт
***


Профиль
Группа: Завсегдатай
Сообщений: 1655
Регистрация: 26.7.2005

Репутация: 17
Всего: 43
Цитата

Чувствую себя индусом

Главное, не используйте этот жаргон - "индус", "быдлокод". Это дурной тон. И все получится. 
PM MAIL   Вверх
Samotnik
Дата 15.6.2011, 17:22 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Super star !
****


Профиль
Группа: Awaiting Authorisation
Сообщений: 7192
Регистрация: 4.11.2006
Где: Минск City

Репутация: 8
Всего: 191
Цитата(COVD @  15.6.2011,  15:49 Найти цитируемый пост)
Главное, не используйте этот жаргон - "индус", "быдлокод". Это дурной тон. И все получится.  

 smile 
apv1989, на самом деле, всё всегда бывает в первый раз, ненужно заниматься самобичеванием, учись на ошибках, читай литературу, всё получится  smile 
PM MAIL   Вверх
dobrolub
Дата 15.6.2011, 19:10 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Опытный
**


Профиль
Группа: Участник
Сообщений: 385
Регистрация: 18.12.2009
Где: Vancouver, Canada

Репутация: 11
Всего: 16
Код

public interface PhotoProvider {
  byte []getPhoto();
  byte []getSmallPhoto();
}

public class MzsrAddressee implements PhotoProvider {
 ...
}

public class KioskAddressee implements PhotoProvider {
  ...
}

...
  - AbstractAddressee adressee;
...
  + PhotoProvider photoProvider = null;
            if ("mzsr".equals(table))
              photoProvider = (PhotoProvider)service.getMzsrAddressee(id);
            else if ("kiosk".equals(table))
              photoProvider = (PhotoProvider)service.getKioskAddressee(id);

...


Это сообщение отредактировал(а) dobrolub - 16.6.2011, 05:02
PM   Вверх
v2v
Дата 20.6.2011, 18:42 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Эксперт
***


Профиль
Группа: Завсегдатай
Сообщений: 1620
Регистрация: 20.9.2006
Где: Киев

Репутация: 8
Всего: 56
1. Вместо строк смол, биг, киоск и т.д. использовать констатны.
2. Избежать дубликации кода (строчки 25-30 и 35-40 в отдельный метод).



--------------------
java блох
PM   Вверх
Rasool
Дата 22.6.2011, 17:44 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


Шустрый
*


Профиль
Группа: Участник
Сообщений: 58
Регистрация: 4.6.2011

Репутация: нет
Всего: нет
А как присутствующие относятся к книге "Рефакторинг. Улучшение существующего кода" Мартина Фаулера в переводе С.Маккавеева? По ней можно учиться культуре программирования?

Это сообщение отредактировал(а) Rasool - 22.6.2011, 17:44
PM MAIL   Вверх
powerOn
Дата 22.6.2011, 22:27 (ссылка) | (нет голосов) Загрузка ... Загрузка ... Быстрая цитата Цитата


software saboteur
****


Профиль
Группа: Участник
Сообщений: 4367
Регистрация: 7.10.2005

Репутация: 47
Всего: 159
Цитата(Rasool @  22.6.2011,  18:44 Найти цитируемый пост)
А как присутствующие относятся к книге "Рефакторинг. Улучшение существующего кода" Мартина Фаулера в переводе С.Маккавеева?


хорошо, но лучше в оригинале.


--------------------
user posted image нет времени думать - нужно писать КОД!
PM MAIL   Вверх
  
 Ответ в темуСоздание новой темы Создание опроса
Правила форума "Java"
LSD   AntonSaburov
powerOn   tux
javastic
  • Прежде, чем задать вопрос, прочтите это!
  • Книги по Java собираются здесь.
  • Документация и ресурсы по Java находятся здесь.
  • Используйте теги [code=java][/code] для подсветки кода. Используйтe чекбокс "транслит", если у Вас нет русских шрифтов.
  • Помечайте свой вопрос как решённый, если на него получен ответ. Ссылка "Пометить как решённый" находится над первым постом.
  • Действия модераторов можно обсудить здесь.
  • FAQ раздела лежит здесь.

Если Вам помогли, и атмосфера форума Вам понравилась, то заходите к нам чаще! С уважением, LSD, AntonSaburov, powerOn, tux, javastic.
 
0 Пользователей читают эту тему (0 Гостей и 0 Скрытых Пользователей)
0 Пользователей:
« Предыдущая тема | Java: Общие вопросы | Следующая тема »

Подписаться на тему | Подписка на этот форум | Скачать/Распечатать тему


 




[ Время генерации скрипта: 0.0592 ]   [ Использовано запросов: 22 ]   [ GZIP включён ]


Реклама на сайте     Информационное спонсорство

 
 По вопросам размещения рекламы пишите на vladimir(sobaka)vingrad.ru
Отказ от ответственности     Powered by Invision Power Board(R) 1.3 © 2003  IPS, Inc.