Показать сообщение отдельно
Старый 21.03.2013, 10:07
maxkar вне форума Посмотреть профиль Отправить личное сообщение для maxkar Найти все сообщения от maxkar
  № 13  
Ответить с цитированием
maxkar

Регистрация: Nov 2010
Сообщений: 497
Цитата:
Что касается кода, я только черновые набросочки сделал, но это не окончательный вариант
Жесть. 100% жесть. Вам не код нужно разрабатывать. Вам сначала нужно удобный API для вашей библиотечеки разработать. Чтобы ею было удобно пользоваться. А уже затем что-то внутри проектировать.

Дальше по коду.
Код AS3:
// Реализация шаблона одиночки
private static var _instance:SocketExchange;
Зачем он здесь? Почему данный класс не предоставляет static accessors к своей функциональности? Он же ничем нужным и полезным не параметризуется. А если параметризуется, автоматически могут существовать несколько экземпляров, параметризованных по-разному. И вообще, посмотрите, зачем же используется singleton. Даже там, где его можно было бы использовать, его стоит на нормальный dependency injection переделать.

Код AS3:
// Реализация шаблона наблюдателя
private var mySub:ISubject;
Не было никогда наблюдателя в сетевых обменах. Не было, нет и не будет. Потому что там нет "состояния" в явном виде. Там есть события. Поэтому максимум, что можно сделать - это message bus. Хотя вру, я делал что-то подобное на observer, но это было для состояния одного конкретного обмена, а не для всего уровня коммуникаций. Кстати, а что должен делать клиент, чтобы какой-нибудьб справочник с сервера подгрузить? И зачем там вообще интерфейс, если реализация примерно одна предполагается и не инжектится снаружи?

Код AS3:
// Парсит объекты из XML в Object
this.parsingXMLtoObject = new ParsingXMLtoObject();
Гениально! Если мне захочется использовать этот обмен в другом проекте, мне нужно будет его исходный код менять? Ведь объекты в другом приложении другие, а я люблю их иметь строго типизированными...

Код AS3:
SocketExchange._instance = new SocketExchange(new PrivateClass());
А смысл? Я вам туда null передам и будет у меня второй SocketExchange. И ничего даже не сломается. И даже в некоторых случаях это будет единственным нормальным вариантом использования этой библиотеки (две подписки на "одно и то же" сообщение).

Код AS3:
// Уведомление наблюдателя
function SetParam(obserName:String, objectParam:Object):void;
Почему не notifyObserver то???

Код AS3:
// Реализует процесс подписки
public function AddObserver(obserName:String,obserFunc:Function):void {
  this.observers[obserName] = obserFunc;
}
Угу. Т.е. только один обсервер для события. А если мне нужно два?

Код AS3:
for(var notify:String in this.observers) {
  // Применяется метод update из интерфейса Observer
  if (notify == this.observerName)
А сразу по this.observerName в this.observers не пойти? В чем глубокий смысл то этого цикла?

Код AS3:
this.observerName = obserName;
this.objectParam = objectParam;
this.NotifyObserver();
Обмен данными через поля класса вместо параметров. Типичный антипаттерн. Выпиливается при первом же рефакторинге (и заменяется на параметры). Потому что отлаживать такое счастье тяжелее, чем передачу параметров. И тестировать отдельные функции проще, чем объекты с состоянияем.

Цитата:
(собственную библиотечку, которую я ранее уже описывал вот в этом сообщении)
Эта библиотечка заслуживает отдельного сообщения. Ну неудобна она и через нетипизированные объекты работает. Все, что она делает, можно предоставить вовне в виде одного статического метода с правильным набором параметров (ну почти, там некоторые хитрые сценарии чуть по-другому обрабатываются).

Код AS3:
internal function set InputData(inputData:Function):void
Вы себя на место пользователя вашей библиотеки ставили? Почему функция "начать загрузку" у вас называется "установить входные данные" и в качестве входных данных устанавливается функция? Ведь не понятно же...

Код AS3:
private function TextError():String
А это должно быть здесь? При переносе этой библиотеки в другой проект мне придется еще и сообщения править. Я всегда думал, что конкретное форматирование ошибок - дело UI-части, а не самого транспорта. А на уровне транспорта нужно предоставить способ различать разные коды ошибок.

Код AS3:
// желательно применить шаблон декоратор к объекту this.xmlDataObject
Зачем там декоратор? Что он будет добавлять?

Код AS3:
var xmlData:XML = XML(e);
О как. А если там не XML? Банальный текст, например (ну не работает сервер)? У вас ведь все умрет (в обычном плеере). Ошибка куда-нибудь улетит и больше не вернется.

Код AS3:
Парсинг принимаемых данных
Почему класс не параметризуется внешним парсером - не понятно. Хотя нет, наверное, понятно. Это из-за излишней любви к синглтонам и неумения собирать граф служб во время запуска программы. У этого класса уже есть ответственность (обработка ошибок транспортного уровня), поэтому парсинг можно бы куда-нибудь в другое место вынести. Можно еще этому классу добавить обработку логических ошибок (внутренняя ошибка сервера, данные не распарсились).

Ваши парсеры не валидируют структуру. Т.е. у вас все поля опциональные (и весь остальной код к этому должен быть готов).

P.S. Я не помню, что у вас в той библиотеке происходит при двух параллельных запросах (т.е. второй приходит тогда, когда первый еще не отработал). Вроде бы что-то нехорошее...