|
|
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Сделал тестовое задание - записная книжка. Функциональность приложения: добавлять новый контакт, удалять контакт, искать контакт по имени и просматривать все контакты. Запись контакта должна содержать имя, номер телефона, имейл. Я добавил ещё добавление имейла и телефона к существующему контакту. Данные сохраняются в XML. Ответили отказом :-( Покритикуйте, пожалуйста, код, что нужно в нем изменить, чтоб улучшить? меню программы: Код: 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. Создаются объекты Contact, которые потом добавляются в XML: Код: 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. Интерфейс содержит название файла: Код: java 1. 2. 3. 4. Классы Email, Name и PhoneNumber при помощи композиции объединяются в классе Contact для создания нового объекта-записи (я решил так будет лучше, если нужно будет добавить ещё адрес, к примеру, то просто создается отдельный новый класс Adress и включается потом в класс Contact): Код: java 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. Код: java 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. Код: java 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. Класс Helper, который работает с XML: Код: 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. 152. 153. 154. 155. 156. 157. 158. 159. Класс для работы с XML (для того, чтобы не вызывать несколько раз одни и те же куски кода, после записи/удаления из Notebook.xml): Код: java 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. 17. 18. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.10.2013, 21:45:08 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991, много замечаний, сразу видно что не хватает опыта практического. Не скажу что код полный отстой, сам так первые программы писал. Дерзай и все получится. Из основных 1) заводить интерфейс ради константы - моветон, сразу видно что вы не понимаете что такое интерфейс 2) switch замените на паттерн команда(почитайте про него) 3) вы на каждый чих пересоздаете и перегружаете xml - это непроизводительно, подумайте как закэшить 4) классы Name Email - нафиг, хватит обычных стринговых переменных ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 18.10.2013, 22:55:03 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991что нужно в нем изменить, чтоб улучшить? используйте JAXB Код: 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. заворачивайте код в спойлер ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 19.10.2013, 09:27:48 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991, Для работы с командной строкой используйте apache cli. А куда и на какую позицию ходили? Для джуниора код нормальный, в принципе. Видал я и от людей с опытом лет 5-10 говнокод копипастный и похлеще. Не так уж и все плохо. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 19.10.2013, 11:32:16 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
забыл никВитяй1991, много замечаний, сразу видно что не хватает опыта практического. Не скажу что код полный отстой, сам так первые программы писал. Дерзай и все получится. Из основных 1) заводить интерфейс ради константы - моветон, сразу видно что вы не понимаете что такое интерфейс 2) switch замените на паттерн команда(почитайте про него) 3) вы на каждый чих пересоздаете и перегружаете xml - это непроизводительно, подумайте как закэшить 4) классы Name Email - нафиг, хватит обычных стринговых переменных Большое спасибо, мотаю на ус! ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 20.10.2013, 16:22:01 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Usman, спасибо) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 20.10.2013, 16:22:24 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
mr_goodkat, да, это на джуниора ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 20.10.2013, 16:23:37 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991mr_goodkat, да, это на джуниора Ну если на джуниора - код нормальный имхо, не знаю что там так сильно не понравилось. В общем пишите, пишите... опыт придет, код для джуниора неплохой ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 20.10.2013, 20:49:38 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
В блоке switch много повторяющегося кода. Можно рефакторить. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 20.10.2013, 22:36:51 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991, прикольно сам недавно начал java изучать поковыряю твой код ))) ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 12:02:39 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
В HELPer как-то всё избыточно сложно. Хочется универсального подхода. Больше Digester, больше XPath. Больше умных языков выборки. Если будут проблемы скорости то их надо решать по мере возникновения. А сразу пытаться проектировать оптимизации - это шаг к неоправданному усложнению. И нет смысла создавать entites для Email, Name, Phone. Это переизбыток объектного проектирования. Достаточно их сделать строками и просто поместить в Contact. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 13:38:38 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991, Напрашивается сразу же реализация полиморфной комманды и выкидывание CASE-а по её типу. Дальше не читал. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 14:35:54 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
GaraZ, удачи!) вот исходники, если нужны ещё ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 18:54:05 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
maytonИ нет смысла создавать entites для Email, Name, Phone. Это переизбыток объектного проектирования. Достаточно их сделать строками и просто поместить в Contact. А был бы смысл, если б планировалось потом, добавить кроме просто телефона, ещё тел. домашний, рабочий, мобильный или адрес - домашний, рабочий и т.д., ну, чтобы всё в одну кучу не мешать, а по классам раскидать? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 18:59:55 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991maytonИ нет смысла создавать entites для Email, Name, Phone. Это переизбыток объектного проектирования. Достаточно их сделать строками и просто поместить в Contact. А был бы смысл, если б планировалось потом, добавить кроме просто телефона, ещё тел. домашний, рабочий, мобильный или адрес - домашний, рабочий и т.д., ну, чтобы всё в одну кучу не мешать, а по классам раскидать? Вам надо учиться мыслить сущностями. По вашему вопросу - если в программе предполагается использовать сущность адрес, как-то найти кто жтвет в таком-то доме, в такой то квартире, в таком то городе - то имеет смысл вычленить класс Adress, если вам нужна тупо строка Россия, Кемеров, Ленина 17-52 для показа на экране - тогда смысла нет. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 19:09:49 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991maytonИ нет смысла создавать entites для Email, Name, Phone. Это переизбыток объектного проектирования. Достаточно их сделать строками и просто поместить в Contact. А был бы смысл, если б планировалось потом, добавить кроме просто телефона, ещё тел. домашний, рабочий, мобильный или адрес - домашний, рабочий и т.д., ну, чтобы всё в одну кучу не мешать, а по классам раскидать? Есть разные уровни генерализации или абстракции. Тебе никто не запрещает забить в классе Map<String,String> attributes и убить сразу всех зайцев. Ты получишь универсальное решение где будут телефоны, аськи, скайпы мессенджеры вконтактики и прочие фейсбуки. Но в том виде в котором оно щас реализовано никакого смысла в инкапсулировании Name, Phone, Email нету. У них нет общего интерфейса и рефлексию ты не используешь. И аргумент "чтобы всё в одну кучу не мешать, а по классам раскидать" я принимаю когда есть метрики к примеру размера исходного кода. Есть некий смысл не делать размер модуля более 7000 строк (с) Старина Вирт и не мельчить сущности до 1 атрибута в каждой. Это тоже экстремизм проектирования. В конце концов Java-девелопер получает деньги за программный продукт а не за количество классов и геттеров с сеттерами. По сабжу они считаются "шумным" и бесполезным кодом. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 21.10.2013, 19:14:30 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
Витяй1991, спасиб )) я еще вчера скомпилил и поковырял ^_^ я пишу прогу и настройки в ней сохраняю в ini файле вот думаю переписать чтобы хранить в xml ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 22.10.2013, 10:22:29 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
забыл ник... 2) switch замените на паттерн команда(почитайте про него) ... Вы можете объяснить чем здесь оправдано применения данного паттерна, чем оно лучше например простого набора статичных методов отвечающих за каждую отдельную операцию: addContact(...) addEmail(...) ... ? Или это просто блеснуть перед работодателем собственно самим знанием этого паттерна? ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 22.10.2013, 10:43:26 |
|
||
|
Покритикуйте код
|
|||
|---|---|---|---|
|
#18+
MaxNevermindИли это просто блеснуть перед работодателем собственно самим знанием этого паттерна? "блеснуть" не получиться, если работодатель умеет отличать "команду" от "стратегии" и знает основное назначение обоих. ... |
|||
|
:
Нравится:
Не нравится:
|
|||
| 22.10.2013, 10:46:55 |
|
||
|
|

start [/forum/topic.php?fid=59&gotonew=1&tid=2128370]: |
0ms |
get settings: |
6ms |
get forum list: |
10ms |
check forum access: |
2ms |
check topic access: |
2ms |
track hit: |
139ms |
get topic data: |
7ms |
get first new msg: |
3ms |
get forum data: |
1ms |
get page messages: |
31ms |
get tp. blocked users: |
1ms |
| others: | 182ms |
| total: | 384ms |

| 0 / 0 |
