ReSQL.ru
     
powered by simpleCommunicator - 2.0.61     © 2026 Programmizd 02
Мобильная версия    Контакт    Правила    FAQ    Помощь
Целевая тема:
Создать новую тему:
Автор:
Закрыть
Цитировать
Доб. в избранное | Игнор. тему | Прикреп. тему | Пометить прочит. / непрочит. | Фильтр
Форумы / Java [игнор отключен] [закрыт для гостей] / рефакторинг
25 сообщений из 107, страница 1 из 5
  1  все
рефакторинг
    #39334807
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
Доброго времени суток..
Имеется некий проект предназначенный для более глубокого понимания программирования под web и личного пользования. В нем есть некий сервлет и jsp`шка. Сразу скажу это не тестовое задание куда-то, а все для себя любимого.

Я начал читать книгу Роберта Мартина "Чистый код", и мои подозрения что мой код является говнокодом переросли в уверенность, что мой код даже хуже, чем я думал.

И вот я сейчас в сомнениях, пробовать писать все заново, или же рефакторить существующий код по мере чтения сенсея, Мартина, и снова и снова.. Скорее всего склонюсь к рефакторингу существующего, так как основная задача на этот кусок была познать основы hibernate, а начав переписывать хочу сделать это на Spring. Но перед спрингом хочу опять таки прописать все ручками. что бы не стать ботом пишущем на 2 фреймоворках хэлло ворды, и не понимающим сути происходящих процессов..

Впрочем не буду утомлять фантазиями очередного недоджуна, а ближе к делу.. Ближе к делу, подскажите как лучше отрефакторить код, в чем проблемы, в коде, в выборе технологий, что и как было бы более красиво и подходяще.. Сам результат, вот ссылка на картинку , меня полностью устраивает, а вот код не очень..

Собственно сам код:


 
servlet
Код: 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.
package modules.kdp.servlets;

import modules.general.Factory;
import modules.kdp.dao.BookKDPDao;
import modules.kdp.dto.BookKDP;

import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.sql.SQLException;
import java.text.SimpleDateFormat;
import java.util.*;

@WebServlet("/promoOld.do")
public class PromoCalendarOld extends HttpServlet {

    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        Factory factory = Factory.getInstance();
        SimpleDateFormat sdf = new SimpleDateFormat("MMM dd, YYYY", Locale.ENGLISH);
        Date date = new Date(sdf.format(new Date()));
        Calendar c = Calendar.getInstance();
        c.setTime(date);
        c.add(Calendar.DATE, -2);
        date = c.getTime();
        c.add(Calendar.DATE, 7);
        Date date2 = c.getTime();
        try {
            BookKDPDao bookKDPDao = factory.getBookKDPDao();
            List<BookKDP> promo = bookKDPDao.getBooksKDPPromoRangeDate(date, date2);
            request.setAttribute("bookAuthors", promo);
        } catch (SQLException e) {
            response.sendRedirect("error.do");
            e.printStackTrace();
        }
        request.getRequestDispatcher("WEB-INF/views/promoCalendarOld.jsp").forward(request, response);
    }

    @Override
    protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        resp.setContentType("text/html");

    }
}



 
JSP
Код: 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.
67.
68.
69.
70.
71.
72.
73.
74.
75.
76.
77.
78.
79.
80.
81.
82.
83.
84.
85.
86.
87.
88.
89.
90.
91.
92.
93.
94.
95.
96.
97.
98.
99.
100.
101.
102.
103.
104.
105.
106.
107.
108.
109.
110.
111.
112.
113.
114.
115.
116.
117.
118.
119.
120.
121.
122.
123.
124.
125.
126.
127.
128.
129.
130.
131.
132.
133.
134.
135.
136.
137.
138.
139.
140.
141.
142.
143.
144.
145.
146.
147.
148.
149.
150.
151.
<%@ page contentType="text/html;charset=UTF-8" language="java"%>
<%@ include file = "check_user.jsp"%>
<%@ include file = "header.jsp"%>
<%@page import="java.util.List"%>
<%@ page import="modules.kdp.dto.BookKDP" %>
<%@ page import="java.util.Locale" %>
<%@ page import="java.text.SimpleDateFormat" %>
<%@ page import="java.util.Date" %>
<%
    SimpleDateFormat sdf = new SimpleDateFormat("MMM dd, YYYY", Locale.ENGLISH);
    SimpleDateFormat sdf2 = new SimpleDateFormat("EEEEE / MMM dd", Locale.ENGLISH);
    Date date = new Date(sdf.format(new Date()));
    Calendar cl = Calendar.getInstance();
    cl.setTime(date);
    cl.add(Calendar.DATE, -2);
%>

<div class="container-fluid">
    <table  style="width:100%" border="1" align="center" frame="void">
        <thead>
        <tr bgcolor="black"  style="color: white;" align="center">
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
            <%cl.add(Calendar.DATE, 1);%>
            <th style="width:14.3%; text-align: center;"><%=sdf2.format(cl.getTime()) %></th>
        </tr>
        </thead>
        <tbody>
        <tr valign="top">
            <td>
                <%
                    cl.setTime(date);
                    cl.add(Calendar.DATE, -2);
                    List<BookKDP> books = (List<BookKDP>) request.getAttribute("bookAuthors");
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
            <td>
                <%  cl.add(Calendar.DATE, 1);
                    for(BookKDP bookKDP: books) {
                        if(cl.getTime().getTime() == bookKDP.getPromo1().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo2().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo3().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo4().getTime()
                                || cl.getTime().getTime() == bookKDP.getPromo5().getTime()
                                ){
                %>
                <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>
                <%  }};  %>
            </td>
        </tr>
        </tbody>
    </table>
</div>
<%@ include file = "footer.jsp"%>

...
Рейтинг: 0 / 0
26.10.2016, 21:22
| Ответить | Цитировать | Написать  
рефакторинг
    #39334815
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
какой ide пользуешься.?
инклуд лучше делать фалы jspf
...
Рейтинг: 0 / 0
26.10.2016, 21:32
| Ответить | Цитировать | Написать  
рефакторинг
    #39334816
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
для даты лучше использовать фишки даты java8
...
Рейтинг: 0 / 0
26.10.2016, 21:34
| Ответить | Цитировать | Написать  
рефакторинг
    #39334819
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
стили задавать в файлах css
...
Рейтинг: 0 / 0
26.10.2016, 21:36
| Ответить | Цитировать | Написать  
рефакторинг
    #39334822
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
обработку событий лучше перенести в функции, а в элементах указывать имя функции.
...
Рейтинг: 0 / 0
26.10.2016, 21:41
| Ответить | Цитировать | Написать  
рефакторинг
    #39334823
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
в именах классов, айдишников в качестве разделителя лучше использовать _ , а не -
...
Рейтинг: 0 / 0
26.10.2016, 21:43
| Ответить | Цитировать | Написать  
рефакторинг
    #39334829
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
Спасибо, так быстро много ответов :)
ide - IntelliJ IDEA 15
jspf - почитаю, что это и как работать

Даты в java8 хотел, но так как сделал на hibernate 4, то они не хотят нормально работать с DB, а при обновлении на hibernate 5, все полетело, целый день пытался вернуть, но в итоге все равно восстановился с бекапа, решил java 8 и hibernate 5 оставить на следующий раз, когда буду переписывать этот проект под spring. Если такой подход не верен, напишите, буду пробовать перейти на этом проекте..

стили - принято, сделаю

имена классов - это bootstrap

о обработке событий немного не понял, имеется ввиду функционал взятия из базы данных в сервлете вынести из doGet в отдельный метод?
...
Рейтинг: 0 / 0
26.10.2016, 21:57
| Ответить | Цитировать | Написать  
рефакторинг
    #39334832
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
про хибермайт - я против этой прокладки, лишнее звено.
<input onclick= "имя_функции();" >


<script>
function имя_функции(){


}

</skript>
...
Рейтинг: 0 / 0
26.10.2016, 22:07
| Ответить | Цитировать | Написать  
рефакторинг
    #39334834
Фотография Petro123
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Petro123
Участник
liberum,
кучу кода занимает таблица. Можно заменить на компонент библиотечную.
...
Рейтинг: 0 / 0
26.10.2016, 22:12
| Ответить | Цитировать | Написать  
рефакторинг
    #39334838
Фотография Паша01
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Паша01
Участник
liberum,

Привет, я бы тоэе кое-что посоветовал.
1) Вынеси "MMM dd, YYYY" в константу в отдельный класс, типо DateFormat. Она может много ещё где использоваться.
2) Из jsp выпили все скриплеты. Если надо что-то высчитать, то делай это в других классах, а на jsp передавай потом результат. Обработать результат на самой jsp поможет, к примеру, jstl (погугли). И все, никаких вычислений на jsp. Почему? Потому что ты одну и ту же логику можешь использовать на нескольких jsp, ты же не будет дублировать код много раз?
3) Выноси все аттрибуты style в css файл. Придумай имена классов.
...
Рейтинг: 0 / 0
26.10.2016, 22:18
| Ответить | Цитировать | Написать  
рефакторинг
    #39334839
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
вадяliberum,
про хибермайт - я против этой прокладки, лишнее звено.


Использую исключительно с целью изучения, голый jdbc использовал ранее, знание фреймворков повышает шансы на трудоустройство..

перенести onclick в функцию - принял


Заменить таблицу на компонент библиотечную это как?
...
Рейтинг: 0 / 0
26.10.2016, 22:20
| Ответить | Цитировать | Написать  
рефакторинг
    #39334841
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
Вынести SimpleDataFormat в константу - принято, сам думал, использую много где.
jstl - погуглю вместе с jspf
...
Рейтинг: 0 / 0
26.10.2016, 22:25
| Ответить | Цитировать | Написать  
рефакторинг
    #39334843
Фотография Petro123
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Petro123
Участник
liberumЗаменить таблицу на компонент библиотечную это как?
типа http://www.jtable.org/
...
Рейтинг: 0 / 0
26.10.2016, 22:28
| Ответить | Цитировать | Написать  
рефакторинг
    #39334844
Фотография fixxer
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
fixxer
Участник
liberumВынести SimpleDataFormat в константу - принято, сам думал, использую много где.
jstl - погуглю вместе с jspf

SimpleDataFormat не надо в константу, он не tread safe. Вынесите туда строку с форматом. По существу вопроса - переписывать, потому что рефакторить там надо решительно все.
...
Рейтинг: 0 / 0
26.10.2016, 22:33
| Ответить | Цитировать | Написать  
рефакторинг
    #39334853
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
fixxerSimpleDataFormat не надо в константу, он не tread safe. Вынесите туда строку с форматом. По существу вопроса - переписывать, потому что рефакторить там надо решительно все.
А если не в константу в класс, с методом типа getDate, которая будет возвращать клон? Часто нужна дата округленная до начала суток, что собственно и решаю конструкцией:
Код: java
1.
2.
        SimpleDateFormat sdf = new SimpleDateFormat("MMM dd, YYYY", Locale.ENGLISH);
        Date date = new Date(sdf.format(new Date()));



Рефакторить хочу что бы знать как писать правильно, если сразу начну переписывать проект, чувствую напишу тот же говнокод, только в профиль..
...
Рейтинг: 0 / 0
26.10.2016, 22:55
| Ответить | Цитировать | Написать  
рефакторинг
    #39334855
liberum
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
liberum
Гость
Petro123liberumЗаменить таблицу на компонент библиотечную это как?
типа http://www.jtable.org/

Слишком избыточно.. Кроме того не уверен, что смогу ее заполнить. Заполняется сначала столбик понедельника, потом вторника и т.д. В моем случаи вид таблицы делается с помощью css, а сама таблица имеет всего 2 строки, первая - заглавие, вторая данные на весь день недели.
...
Рейтинг: 0 / 0
26.10.2016, 23:08
| Ответить | Цитировать | Написать  
рефакторинг
    #39334908
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
liberum,
Код: html
1.
      <tr valign="top">


https://webref.ru/html/td/valign

Код: html
1.
2.
3.
 <div style="width:100%;overflow:hidden;">
                    <input type="text" style="width: 100%; background: #f1f1f1;" title=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %> " ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')" readonly="" value=" <%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
                </div>



Код: html
1.
title

- https://webref.ru/html/title

Код: html
1.
input

- https://webref.ru/html/input

вообще инпут в div непонятно для чего, ondblclick можно назначить и самому div.

https://webref.ru/css
https://webref.ru/html
http://frontender.info/a-guide-to-flexbox/
https://learn.javascript.ru/
http://jquery.page2page.ru/index.php5/Заглавная_страница

по логике js
https://www.amazon.com/dp/<%=bookKDP.getAsin() лучше вынести в div
Код: html
1.
2.
3.
<div class='vvv' ondblclick="window.open('https://www.amazon.com/dp/<%=bookKDP.getAsin()%>','_blank')">
<%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
</div>



но ещё лучше
Код: html
1.
2.
3.
<div class='vvv' ondblclick="fff(%=bookKDP.getAsin()%>);')">
<%= bookKDP.getBookAuthorsWriter().getBook().getTitle() %>">
</div>



Код: javascript
1.
2.
3.
4.
5.
<script>
function fff(p){
window.open('https://www.amazon.com/dp/'+p,'_blank');
}
</script>



по входу с паролём http://findevelop.blogspot.ru/2013/10/web-spring-security-100.html
...
Рейтинг: 0 / 0
27.10.2016, 05:31
| Ответить | Цитировать | Написать  
рефакторинг
    #39334909
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
ЗЫ
имена взяты с потолка :)
проверить правильность на использование кавычек (двойных и одинарных) к сожалению тут править не сделано
...
Рейтинг: 0 / 0
27.10.2016, 05:34
| Ответить | Цитировать | Написать  
рефакторинг
    #39334933
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
вадя,

Ну, кто же в 2016м году скриплеты использует? Для доступа-то к свойствам. EL в JSP уже достаточно хорош чтобы всё на нём писать. Скриплеты это табу. Только в исключительных случаях можно применить, если они действительно упрощают решение.
...
Рейтинг: 0 / 0
27.10.2016, 08:37
| Ответить | Цитировать | Написать  
рефакторинг
    #39334940
забыл ник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
забыл ник
Участник
Именовать переменные таким образом Date date2 = c.getTime(); очень плохо, вы скоро забудете чем отличается date от date2, к тому же стороннему человеку решительно непонятно о чем идет речь. Гораздо лучше называть их как Date startDate например.
Далее, в сервлете в идеале должно быть только то что связано с HTTP, как то - берем параметры из реквеста и преобразуем их в некий объект, валидация этих параметров и всякие sendRedirect forward и тд. Все остальное выносится в отдельный сервис, например DBService.
...
Рейтинг: 0 / 0
27.10.2016, 09:01
| Ответить | Цитировать | Написать  
рефакторинг
    #39334962
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
liberum,

Начните с чтения о таких понятиях как Abstract Layer, Model View Controller и Domain Model.
Вам нужно четко понимать какая строка кода относитя к представлению, какая к модели предметной области, а какая просто обслуживает HTTP. И эти три вещи надо раздести в разные классы и стараться не смешивать, следуя Single Responsibility Principle.

Не используйте скриплеты. В идеале надо JSP заменить на какой-нибудь FreeMarker или Thymeleaf. Но можно использовать и JSP, но разобраться с Expression Language и JSTL.

Каждый раз когда вы копируете кусок кода, бейте себя по рукам и ищите способ изменить код. В JSTL есть тэги для циклов, например. А вы накопипастили 6 одинаковых пар строк. Ваши условия if(cl.getTime().getTime() == bookKDP.getPromo1().getTime() за гранью по двум причинам.
1. Почему 1-2-3-4-5 это разные методы? ПОчему не массив? Почему не список?
2. Каждый раз когда вы пишете bookKDP.getPromo2().getTime() это нарушение инкапсуляции. В идеале дожно быть bookKDP.getPromoTime().
bookKDP.getBookAuthorsWriter().getBook().getTitle() это bookKDP.getBookTitle();


"Чистый код" - правильная книга. Дочитайте и начните применять.
...
Рейтинг: 0 / 0
27.10.2016, 09:29
| Ответить | Цитировать | Написать  
рефакторинг
    #39334969
Фотография Petro123
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Petro123
Участник
Потоки потоками, но 10 строк кода для даты это чересчур).
...
Рейтинг: 0 / 0
27.10.2016, 09:36
| Ответить | Цитировать | Написать  
рефакторинг
    #39334973
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Blazkowicz
Участник
liberum
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
        Factory factory = Factory.getInstance();
        SimpleDateFormat sdf = new SimpleDateFormat("MMM dd, YYYY", Locale.ENGLISH);
        Date date = new Date(sdf.format(new Date()));
        Calendar c = Calendar.getInstance();
        c.setTime(date);
        c.add(Calendar.DATE, -2);
        date = c.getTime();
        c.add(Calendar.DATE, 7);
        Date date2 = c.getTime();



И ещё важный момент. Уверен что в книгде об этом тоже было. Научитесь создавать больше методов. Вот с ходу понять что тут происходит - решительно не возможно. Но если вы сделаете этому куску кода extractMethod и дадите методу внятное объясняющее имя, то это значительно повысит читаемость вашего кода.
...
Рейтинг: 0 / 0
27.10.2016, 09:44
| Ответить | Цитировать | Написать  
рефакторинг
    #39334984
вадя
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
вадя
Участник
Blazkowiczвадя,

Ну, кто же в 2016м году скриплеты использует? Для доступа-то к свойствам. EL в JSP уже достаточно хорош чтобы всё на нём писать. Скриплеты это табу. Только в исключительных случаях можно применить, если они действительно упрощают решение.
зачем городить что-то если можно обойтись простым?
это табу придумано непонятно кем и для чего, а потом его теражируют...
...
Рейтинг: 0 / 0
27.10.2016, 09:55
| Ответить | Цитировать | Написать  
рефакторинг
    #39334987
uid unique
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
uid unique
Участник
Blazkowiczliberum
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
        Factory factory = Factory.getInstance();
        SimpleDateFormat sdf = new SimpleDateFormat("MMM dd, YYYY", Locale.ENGLISH);
        Date date = new Date(sdf.format(new Date()));
        Calendar c = Calendar.getInstance();
        c.setTime(date);
        c.add(Calendar.DATE, -2);
        date = c.getTime();
        c.add(Calendar.DATE, 7);
        Date date2 = c.getTime();



И ещё важный момент. Уверен что в книгде об этом тоже было. Научитесь создавать больше методов. Вот с ходу понять что тут происходит - решительно не возможно. Но если вы сделаете этому куску кода extractMethod и дадите методу внятное объясняющее имя, то это значительно повысит читаемость вашего кода.

У меня тоже вынесло мозг от операций с датой... по моему можно упростить в одну строчку.
Календарь и старые форматтеры желательно заменить joda time , в 8 джаве этот пакет уже влючен, он и пошустрее и thread safe в отличие от Calendar.
...
Рейтинг: 0 / 0
27.10.2016, 09:57
| Ответить | Цитировать | Написать  
25 сообщений из 107, страница 1 из 5
  1  все
Форумы / Java [игнор отключен] [закрыт для гостей] / рефакторинг
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение   Полит. конфиден.  
NoSQL.ru       Кролег: Проекты, Новости, Чат       РЕД ФОРУМ       Фотоальбом: Участники, Встречи
Закрыть
start [/forum/topic.php?fid=59&msg=39334984&tid=2123546]:
 0ms
get settings:
 8ms
get forum list:
 15ms
check forum access:
 4ms
check topic access:
 4ms
track hit:
 38ms
get topic data:
 12ms
get forum data:
 3ms
get page messages:
 85ms
get tp. blocked users:
 2ms
others: 195ms
total:  366ms
созд. / загр.: 366ms
x
x
Закрыть


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