Гость
Войти | Профиль | Очистить
Целевая тема:
Создать новую тему:
Автор:
Действия ...
Форумы / Java [игнор отключен] [закрыт для гостей] / рефакторинг / 25 сообщений из 107, страница 1 из 5
  1  все
26.10.2016, 21:22
    #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:32
    #39334815
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
какой ide пользуешься.?
инклуд лучше делать фалы jspf
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 21:34
    #39334816
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
для даты лучше использовать фишки даты java8
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 21:36
    #39334819
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
стили задавать в файлах css
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 21:41
    #39334822
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
обработку событий лучше перенести в функции, а в элементах указывать имя функции.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 21:43
    #39334823
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
в именах классов, айдишников в качестве разделителя лучше использовать _ , а не -
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 21:57
    #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, 22:07
    #39334832
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
про хибермайт - я против этой прокладки, лишнее звено.
<input onclick= "имя_функции();" >


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


}

</skript>
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 22:12
    #39334834
Petro123
Petro123
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
liberum,
кучу кода занимает таблица. Можно заменить на компонент библиотечную.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 22:18
    #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:20
    #39334839
liberum
liberum
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
вадяliberum,
про хибермайт - я против этой прокладки, лишнее звено.


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

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


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

SimpleDataFormat не надо в константу, он не tread safe. Вынесите туда строку с форматом. По существу вопроса - переписывать, потому что рефакторить там надо решительно все.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
26.10.2016, 22:55
    #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, 23:08
    #39334855
liberum
liberum
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
Petro123liberumЗаменить таблицу на компонент библиотечную это как?
типа http://www.jtable.org/

Слишком избыточно.. Кроме того не уверен, что смогу ее заполнить. Заполняется сначала столбик понедельника, потом вторника и т.д. В моем случаи вид таблицы делается с помощью css, а сама таблица имеет всего 2 строки, первая - заглавие, вторая данные на весь день недели.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
27.10.2016, 05:31
    #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:34
    #39334909
вадя
вадя
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
ЗЫ
имена взяты с потолка :)
проверить правильность на использование кавычек (двойных и одинарных) к сожалению тут править не сделано
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
27.10.2016, 08:37
    #39334933
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
рефакторинг
вадя,

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

Ну, кто же в 2016м году скриплеты использует? Для доступа-то к свойствам. EL в JSP уже достаточно хорош чтобы всё на нём писать. Скриплеты это табу. Только в исключительных случаях можно применить, если они действительно упрощают решение.
зачем городить что-то если можно обойтись простым?
это табу придумано непонятно кем и для чего, а потом его теражируют...
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
27.10.2016, 09:57
    #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
| Ответить | Цитировать | Написать  
  1  все
Форумы / Java [игнор отключен] [закрыт для гостей] / рефакторинг / 25 сообщений из 107, страница 1 из 5
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение  
Полит. конфиден.  
созд. / загр.: 511ms
Закрыть
start [/forum/topic.php?fid=59&mobile=1&tid=2123546]:
 0ms
get settings:
 8ms
get forum list:
 16ms
check forum access:
 3ms
check topic access:
 3ms
track hit:
 149ms
get topic data:
 10ms
get forum data:
 2ms
get page messages:
 80ms
get tp. blocked users:
 1ms
others: 239ms
total:  511ms
x
x
Закрыть


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