|
|
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Привет всем. В java я новичек, но по работе надо написать небольшой servlet, код которого потом будут читать другие люди. Вобщем я наваял этот сервлет и прежде чем сдавать его очень хотелось бы послушать мнения профессионалов о качестве кода. Заранее благодарю за помощь и понимание. // сам сервлет Код: plaintext 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. 152. 153. 154. 155. 156. 157. 158. 159. 160. 161. 162. 163. 164. 165. 166. 167. 168. 169. 170. 171. 172. 173. 174. 175. 176. 177. 178. 179. 180. 181. 182. 183. 184. 185. 186. 187. 188. 189. 190. 191. 192. 193. 194. 195. 196. 197. 198. 199. 200. 201. 202. 203. 204. 205. 206. // доп. классы Код: plaintext 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. Код: plaintext 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 13:24:30 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Я бы разнес логику по разным классам. Советую почитать про MVC, DAO шаблоны. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 13:43:57 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Да тут комментировать и комментировать. 1) Коменты в импортах не нужны. 2) Исползовать /*..*/ в методах вместо // не красиво, тем более для однострочных коментариев 3) if(strAction == "homepage_data") -- ха-ха. А intern я что-то не вижу. 4) Все строки критичные для логики крайне желательно выность в константы с понятными именами. 5) Нет единого стиля в расстановке скобок. А ведь для этого достаточно настроить IDE и попросить её отформатировать код. 6) out.close(); наверное стоило бы поместить в finally 7) getStaticData - плохое имя для метода. Совершенно ни о чем не говорит. Работа с базой вообще не кудышняя. 8) Параметры базы должны быть в конфиге 9) Пул соединений какой-то странный. Для томката и пр серверов достаточно было бы настроить DataSource. Тут какая-то ядерная смесь самостоятельного создания физических соединений и работой с DataSource. Я даже с ходу не пойму что из этих двоих таки реально используется. В общем этот момент показывает полное незнание JDBC. Судя по всему даже лень было прочитать туториалы по теме. 10) SQL запросы в коде не красиво. Нужно выносить в текстовый файл для удобной модификации, либо использовать фреймверки. 11) Отсутствует логирование. 12) Соединения и стейтменты не закрываются. 13) Ну и дизайн в виде лесенки if...else, тоже выдает отсутствие знаний в ООП. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 13:58:36 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
14) "not implemented yet " лучше делать в виде UnsupportedOperationException 15) SELECT id as id, name as name - а для чего такие алиасы? 16) Ошибки в английском(connection polling - голосование за соединение?), небрежное отношение к сообщенийм об ошибках. 17) С коментариями вообще... они есть там где не надо и отсутствуют там где надо. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 14:08:05 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
2 Blazkowicz: спасибо большое за помощь! тол.ко я что-то не понял по поводу 13-го пункта. В getStaticData (действительно надо переименовать) наверное enum можно привестив int и юзать switch, тока я не знаю как :(. Или Вы что-то другое имели в виду? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 14:20:34 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-s2 Blazkowicz: спасибо большое за помощь! тол.ко я что-то не понял по поводу 13-го пункта. В getStaticData (действительно надо переименовать) наверное enum можно привестив int и юзать switch, тока я не знаю как :(. Или Вы что-то другое имели в виду? Нет, switch это тоже самое. мало чем лучше. Вот смотри. Для начала нужно оценить какая вероятность роста количества твоих Action. Судя по всему довольно большая. Теперь подумай. При создании каждого нового action тебе придется менять метод сервлета и метод работы с базой. А когда проект выростет количество Action вообщен может вырости до нескольких десятков, в результате у тебя будет лесенка if...else, на несколько экранов. А для того чтобы внести изменения в один любой action придется перекомпилирвать как работу с базой так и сервлет. Решается это применением GoF паттерна Action/Command или его урезаным вариантом. В кратце, тебе на каждый Action нужно завести отдельный класс. Кстати, строки типа "get_connection" можно поместить в enum. Читай доку по энумам. Потом тебе надо реализовать механизм который по имени Action вытянет нужный экземпляр класса обработчика. И вызвать метод этого обработчика. Этот механизм можно реализовать кучей способов. Через HashMap, reflection, и даже через твой enum. Ведь в enum-е можно определять методы для каждого элемента. Поиск на rsdn к сожалению сломался. Так бы я нашел тебе пример. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 14:35:40 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
BlazkowiczКстати, строки типа "get_connection" можно поместить в enum. Опечатался. Имелось в виду - get_categories ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 14:36:54 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz Решается это применением GoF паттерна Action/Command или его урезаным вариантом. В кратце, тебе на каждый Action нужно завести отдельный класс. У меня почему-то стойкое ощущение, что это State/Strategy. Возможно ошибочное. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 15:12:58 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
В принципе, да. Скорее стратегия. У меня просто от struts устойчивая ассоциация action и command. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 15:37:54 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
а ткните меня носом в простенький пример с этими шаблонами или дайте линк на хороший туториал плз. (мне это надо сегодня до 12-и ночи сделать) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 15:53:11 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-sа ткните меня носом в простенький пример с этими шаблонами или дайте линк на хороший туториал плз. (мне это надо сегодня до 12-и ночи сделать) Гугл в помощь. Я бы на твоем месте поступил так. За оставшееся время нормально переработал работу с базой. А по поводу №13, там где у тебя несколько if..else пишешь камент: //TODO:[krot-s] Refactor to replace if..else chain with separate classes for better expansibility Потому что за JDBC тебя побъют гораздо больше чем за это. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:01:38 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
я бы еще за табуляции вместо пробелов больно бил по почкам :) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:06:32 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Timmя бы еще за табуляции вместо пробелов больно бил по почкам :) ну а табуляция чем не догодила-то? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:22:24 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-s Timmя бы еще за табуляции вместо пробелов больно бил по почкам :) ну а табуляция чем не догодила-то? Раздражает. Смотреть исходники в разных средах неудобно. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:24:17 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Timm wrote: >> ну а табуляция чем не догодила-то? > Раздражает. Смотреть исходники в разных средах неудобно. Да ну ладно, в сравнении с тем, как сделано в "родных" сорцах (4 пробела, потом табуляция, потом табуляция и 4 пробела и т.д.)- это ничего, это легко правится. А вот за смесь и того и того- точно убивать. -- Алексей Posted via ActualForum NNTP Server 1.4 ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:27:19 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Timmя бы еще за табуляции вместо пробелов больно бил по почкам :) а я бы за еще за скобочки на новой строке, а еще за throw new на одной строке, а Exception на другой... Вобщем надло пользоваться автоматическим форматированием, как уже говорили, обычно в нормальных IDE темплейты форматирования соответствуют общепринятым стандартам ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:27:23 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
идея при нажатии на home ставит курсор в начало строки <--> текста в строке, так что без разницы, есть там табы или нет Posted via ActualForum NNTP Server 1.4 ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:36:30 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
y3uа я бы за еще за скобочки на новой строке А вот этого не надо. Такой стиль удобнее для чтения. Об этом даже в Bitter Java написано. Меня после полу-года C# все 4 года явы воротит от явовского стиля скобки на той же строке. Вообще обсуждения табики vs пробелы и куда ставить скобки это древние флеймы. На rsdn несколько лет назад обсуждались. А вывод только один. Ставить можно как угодно. Только надо быть увереным, что все в одном стиле. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 16:58:37 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz y3uа я бы за еще за скобочки на новой строке А вот этого не надо. +1. мне так удобно :) Deadyидея при нажатии на home ставит курсор в начало строки <--> текста в строке, так что без разницы, есть там табы или нет среда != IDE в общем случае. хоть блокнот. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 18:27:09 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz y3uа я бы за еще за скобочки на новой строке А вот этого не надо. Такой стиль удобнее для чтения. Об этом даже в Bitter Java написано. во-первых я сказал "я бы" :), тем более у нас в компании такой стандарт, мы используем смешаный стиль, для "короткого" кода и для длинного... Мало ли что написано в горькой джаве, не стоит канонизировать и превращать в религию простые здравые взгляды на код, к которым нормальный девелопер, если он не конченый раздолбай, приходит самостоятельно примерно через 2 года усиленной работы с кодом и самообразования... ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 20:17:00 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
y3uво-первых я сказал "я бы" :), тем более у нас в компании такой стандарт, мы используем смешаный стиль, для "короткого" кода и для длинного... Мало ли что написано в горькой джаве, не стоит канонизировать и превращать в религию простые здравые взгляды на код, к которым нормальный девелопер, если он не конченый раздолбай, приходит самостоятельно примерно через 2 года усиленной работы с кодом и самообразования... Не надо канонизировать стандарт своей компании. О религии ты тут первый заговорил. У меня, позиция простая - стиль должен быть единым на проект, все равно каким. - скобка на новой строке облегчает чтение. Как ты тут религию с канонами разглядел, не понятно. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 21:27:57 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
BlazkowiczНе надо канонизировать стандарт своей компании.ну да.. лучше канонизировать стандарт самого себя, как ты это очень неплохо показал в этом топике. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 21:40:59 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
учел я замечания Blazkowicz и сделал все в отдельных классах с рефлексией. Может еще раз код выложить, чтобы вы его похаяли (для меня это будет оч. полезно)? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 22:27:22 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Выложи- самому интересно поучиться ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 22:48:34 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz - скобка на новой строке облегчает чтение. ура, давайте флудить! Код: plaintext 1. 2. 3. 4. 5. 6. 7. 8. мне не легче читается, чем Код: plaintext 1. 2. 3. 4. 5. по крайней мере на моем монике на 3 строчки полезного кода больше влазит... Лично я делаю скобку на новой строке только в лучае, если получается какая-то длинная строка, но это бывает очень редко, т.к. я в таких случаях делаю экстракт метод :) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 22:56:46 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
y3u Blazkowicz - скобка на новой строке облегчает чтение. ура, давайте флудить! давайте. Код: plaintext 1. 2. 3. 4. Код: plaintext 1. 2. 3. 4. 5. 6. 7. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 23:20:55 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
swtну да.. лучше канонизировать стандарт самого себя, как ты это очень неплохо показал в этом топике. Лучше обсудить собеседника, чем сказать что-то умное? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 23:53:13 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
y3uмне не легче читается, чем Ну просто железный, аргумент. С чем тут спорить. Мнение таких экспертов как Тейт, конечно же ерунда. Да, кто он такой. "Мне не легче" и это главное. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 17.07.2007, 23:57:19 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-sучел я замечания Blazkowicz и сделал все в отдельных классах с рефлексией. Может еще раз код выложить, чтобы вы его похаяли (для меня это будет оч. полезно)? Интересно, а к чему был этот вопрос? Выложил - давно бы уже обсудили. Можно начать с того что использование рефлекии скорее всего не обосновано в данном случае. Лично я бы предпочел использовать рефлексию в последнюю очередь. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 00:01:43 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
давайте пофлудим wrote: > давайте. На мой взгляд на одной строчке должна быть одна конструкция. И если она помещается на одной строчке- она должна быть на одной. if(...){ - одно по сути ... }else{ - другое ... } - третье. Увы, модный C-like синтаксис... А языках идущих от синтаксиса algol-68 всё понятнее (на примере Ada) if ... then ... else ... end if; Лучше уж писать на Java как на языках с нормальным синтаксисом :) -- Алексей Posted via ActualForum NNTP Server 1.4 ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 07:58:08 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 09:43:47 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
ну вот новая версия. Только с исключениями я перемудрил. что-то не могу по-человечески сделать. может подскажете как лучше всего их обрабатывать? К примеру: приходит запрос на получение каких-то данных из базы. Сначала подгружается соответствующий класс, потом в том классе создается объект для подключения и при подключении возникает exception. Как лучше всего "выплюнуть" это исключение на самый верх для обработки, а то у меня это как-то коряво вышло... Код: plaintext 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. // класс подключения к базе Код: plaintext 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. 152. 153. 154. 155. 156. 157. 158. 159. 160. 161. 162. 163. 164. 165. 166. 167. 168. 169. 170. 171. 172. 173. 174. 175. 176. 177. 178. 179. 180. 181. 182. // класс обработки запрошеного действия Код: plaintext 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. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 10:51:52 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Ну, понеслась по новой. 1) На счет reflection и поиска Action по имени. По хорошему нужно чтобы каждый класс Action реализовывал интерфейс с методом getData. Тогда перед тем как делать getData можно было бы проверить найденый класс реализует этот интерфейс или нет. И вообще можно было бы обойтись без reflection. У тебя был очень хороший enum. С каждым элементом энума можно ассоциировать поля. На твоем бы месте я именно там хранил имя класса-экшна и имя самого экшна. И в энуме был бы метод который по имени экшна возвращал элемент перечисления. А из элемента можно было бы получить класс. (А по хорошему туда можно сразу сложить экземпляры классов, чтобы не делать Class.forName()) 2) Класса работы с базой я вообще не понял. Зачем тебе вариант там где нет datasource? Вот этого "java:comp/env/jdbc/conversion" и вот этого "jdbc:mysql://" не должно быть в коде. Это конфигурационные данные. Вообще нет смысла собирать строку соединения в коде. Обычно она целиком прописывается в конфиге. Имя и пароль не обязательно засовывать в строку соединения. Для них есть второй метод getConnection 3) Код: plaintext 1. 2. 3. 4)В хорошем коде обычный Exception никогда не ловится и никогда не прокидывается. Надо ловить только ожидаемые исключения, а не все подря. Надо прокидывать свои бизнес-исключения. Beware the dangers of generic Exceptions - Java World 5) Зачем для конфига использовать DOM когда с классом Properties было бы гораздо меньше кода? 6) throw new Exception("Failed to loading config file: invalid format", new java.lang.Throwable()); На кой здесь new java.lang.Throwable()? Ты доки по классам когда-нибудь читаешь? И как всегда английский - "Failed to load" 7) При работе с datasource строка Class.forName("org.gjt.mm.mysql.Driver") не нужна. Почему она тогда вне условия? 8) "text/html" разве подходящий MIME тип для JSON? JSON ведь совсем не HTML 9) Над Coding Convention надо ещё поработать. Почему некоторые классы имеют приставку C, а некоторые - нет. Именование переменных и методов не всегда удачное. Экономить букавы в именах не вседа полезно. Чем ближе код к английскому языку тем легче он читается. 10) Для чего иногда используется полное имя класса включающее пакет? 11) Сдвиг для строк какой-то большой, обычно используется 4 пробела. У тебя сколько? Или это парсер так нарисовал? 12) Когда ты работаешь только с интерфейсом Map нет никакого смысла объявлять переменные и возвращаемые типы как HashMap. Мне кажется хорошим стилем использовать наиболее общий интерфейс. Поэтому HashMap достаточно только при создании экземпляра. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 11:51:26 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz 11) Сдвиг для строк какой-то большой, обычно используется 4 пробела. У тебя сколько? Или это парсер так нарисовал? Это как раз дебильный side-effect табуляции. На форуме 1 tab = 8 spaces. smbdyимхо, аргументы аля "мне так удобней" не совсем корректны. Не нужно забывать, что обычно люди работают в комманде. Да и не факт что через пару лет кому то не прийдеца его сапортить. Это был не аргумент, а констатация факта. Впрочем в компании так принято, как раз из-за удобства. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 12:18:29 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz 1) На счет reflection и поиска Action по имени. По хорошему нужно чтобы каждый класс Action реализовывал интерфейс с методом getData. Тогда перед тем как делать getData можно было бы проверить найденый класс реализует этот интерфейс или нет. И вообще можно было бы обойтись без reflection. У тебя был очень хороший enum. С каждым элементом энума можно ассоциировать поля. На твоем бы месте я именно там хранил имя класса-экшна и имя самого экшна. И в энуме был бы метод который по имени экшна возвращал элемент перечисления. А из элемента можно было бы получить класс. (А по хорошему туда можно сразу сложить экземпляры классов, чтобы не делать Class.forName()) вот это я вообще не понял... :( Blazkowicz 5) Зачем для конфига использовать DOM когда с классом Properties было бы гораздо меньше кода? А можно подробней, а то я вчера быстренько пример разбора XML нашел, его и впихнул? Blazkowicz 11) Сдвиг для строк какой-то большой, обычно используется 4 пробела. У тебя сколько? Или это парсер так нарисовал? у меня вообще табуляции везде, а не пробелы. Так что это парсер. p.s. еще раз спасибо за помощь. если какими-либо судьбами занесет в Одессу - с меня пиво (мыло в профиле) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 12:22:14 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-s А можно подробней, а то я вчера быстренько пример разбора XML нашел, его и впихнул? для чтения конфигов обычно использую Digester ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 12:41:58 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
krot-s вот это я вообще не понял... :( http://java.sun.com/docs/books/tutorial/java/javaOO/enum.html Смотри внизу пример с Planet и думай. krot-s А можно подробней, а то я вчера быстренько пример разбора XML нашел, его и впихнул? http://www.exampledepot.com/egs/java.util/Props.html Начиная с Java 5 может работать с XML ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 13:07:09 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Да, и вот все вспоминаю и забываю вписать. В коде отсутствует логирование. По хорошему на уровне логгера DEBUG надо в лог валить и запросы, и результаты в виде JSON. Ну и вообще правильно логировать на всех уровнях ERROR/WARN/INFO/DEBUG ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 14:04:53 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
BlazkowiczДа тут комментировать и комментировать. 10) SQL запросы в коде не красиво. Нужно выносить в текстовый файл для удобной модификации, либо использовать фреймверки. а если благодоря "службам безопасности" работа приложения с текстовыми файлами у заказчика не возможна (реальный случай имелся) или запрос формируется динамически в зависимости от каких-то условий, что тогда делать?) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 14:35:25 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
newjа если благодоря "службам безопасности" работа приложения с текстовыми файлами у заказчика не возможна (реальный случай имелся) или запрос формируется динамически в зависимости от каких-то условий, что тогда делать?) Текстовые файлы находятся не где-то там на диске, а где-то внутри приоложения, рядом с классами, запакованые в jar. Какие тут ещё безопасности? Может и JVM запускать нельзя? Она столько всяких файлов читает аж страшно. Когда запрос формируется динамически, то используются шаблоны. Смотри, например, iBatis и подобные. Зависит конечно от ситуации. Но как минимум имена таблиц и колонок могут изменится. Запросы можно оптимизировать. И выходит для этого всегда придется пересобирать проект (еже ли запросы в коде хранить)? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.07.2007, 15:19:23 |
|
||
|
покритикуйте код и может чего посоветуйте чего?
|
|||
|---|---|---|---|
|
#18+
Blazkowicz newjа если благодоря "службам безопасности" работа приложения с текстовыми файлами у заказчика не возможна (реальный случай имелся) или запрос формируется динамически в зависимости от каких-то условий, что тогда делать?) Текстовые файлы находятся не где-то там на диске, а где-то внутри приоложения, рядом с классами, запакованые в jar. Какие тут ещё безопасности? Может и JVM запускать нельзя? Она столько всяких файлов читает аж страшно. не знаю какие там безапасности, но текстовый файл был не где то там , а как раз в приложении. при попытке чтения был посыл в... о чем сообщал WAS своими логами и его админ. Когда запрос формируется динамически, то используются шаблоны. Смотри, например, iBatis и подобные. Зависит конечно от ситуации. Но как минимум имена таблиц и колонок могут изменится. Запросы можно оптимизировать. И выходит для этого всегда придется пересобирать проект (еже ли запросы в коде хранить)? спасибо, надо глянуть. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 19.07.2007, 09:23:16 |
|
||
|
|

start [/forum/topic.php?all=1&fid=59&tid=2145127]: |
0ms |
get settings: |
12ms |
get forum list: |
22ms |
check forum access: |
5ms |
check topic access: |
5ms |
track hit: |
40ms |
get topic data: |
12ms |
get forum data: |
4ms |
get page messages: |
100ms |
get tp. blocked users: |
2ms |
| others: | 299ms |
| total: | 501ms |

| 0 / 0 |
