Показаны сообщения с ярлыком code smells. Показать все сообщения
Показаны сообщения с ярлыком code smells. Показать все сообщения

вторник, 21 мая 2013 г.

Чудесный ужас №2

Первая серия была тут - http://18delphi.blogspot.com/2013/05/blog-post_6566.html

Я конечно уважаю "Остап Ибрагимыча".

И я могу найти - МИЛЛИОН доводов - почему, код приведённый ниже - написан так, а не иначе. НО! Очень сложно будет убедить меня, что он "не пахнет".


Чудесный ужас №2:
....
    property ActiveControl: TWinControl read FActiveControl write SetActiveControl
      stored IsForm;
...
procedure TCustomForm.Loaded;
var
  Control: TWinControl;
begin
  inherited Loaded;
  if ActiveControl <> nil then
  begin
    Control := ActiveControl;
    FActiveControl := nil;
    if Control.CanFocus then SetActiveControl(Control);
  end;
end;
и ХЕ:
procedure TCustomForm.Loaded;
var
  Control: TWinControl;
begin
  inherited Loaded;
  if (ActiveControl <> nil) and ((Parent = nil) or not (csDesigning in ComponentState)) then
  begin
    Control := ActiveControl;
    FActiveControl := nil;
    if Control.CanFocus then SetActiveControl(Control);
  end;
– называется - "чота мы залатали"... Но глаз - даже не резануло.. :-( Жалко, что у людей не сработало "чувство запаха".
А ведь 80% этого "ужаса" снялось бы введением метода ForceSetActiveControl.
Но у меня есть МНОГО претензий к коду Borland'а в части обработки фокуса, особенно при наличии докинга.
... to be continued ...

P.S. До придумки Apple в xCode - ShowContextMenuForView, BecomeFirstResponder и CanBecomeFirstResponder - этому коду конечно далеко. Apple - "умнее" - он исходные коды не даёт. А то бы - я наверное многое бы там почерпнул....

P.P.S. А для докинга там есть продолжение. ActiveControl - не всегда в итоге устанавливается. Ибо CanFocus не всегда true.

P.P.P.S Опять спросили "ЗАЧЕМ я всё это пишу". Да ни зачем собственно. Просто чтобы привлечь внимание к вещам, которые меня волнуют. Заставить задуматься. А не потому, что я верю в то, что "кто-то исправит" или "кто-то прислушается". Просто есть хороший повод показать - как я бы делал бы и как я бы не делал бы.

пятница, 17 мая 2013 г.

Чудесный ужас (или - все допускают ошибки)...

Нет. Я конечно очень уважаю "Остап Ибрагимыча" (разработчиков Borland'а). И это не пустые слова. Мне до их уровня - ещё расти и расти. Но у меня в голове всё никак не укладывается вот это:


  TControlActionLink = class(TActionLink)
  protected
    FClient: TControl;

...

  TWinControlActionLink = class(TControlActionLink)
  protected
    FClient: TWinControl;

....

- может быть я чего не понимаю или "не так воспитан".

По мне - одноимённые protected поля в соседних классах - это не комильфо. Особенно если они выполняют ОЧЕНЬ СХОДНЫЕ функции. Куда лучше было бы хотя бы назвать одно поле FClientControl, а другое - FClientWinControl. На мой вкус. Не говоря уж о ненужном дублировании данных.  Всегда вообще-то можно приведение типов написать. И проверку в момент присвоения (AssignClient). Не говоря уж о то, что эта ссылка - НИГДЕ не чистится. А тут имеем БОЛЬШИЕ проблемы с Detsroy. Я правда пока ещё не нащупал - какие именно. Нащупаю - обязательно напишу.

Но по мне этот код и производные от него - "дурно пахнут". Простите уж.

Я думал - хоть под XE это переделали... Ничего подобного :-(

(+)


  TToolbarButton97ActionLink = class(TControlActionLink)
  protected
    FClient: TCustomToolbarButton97;


- получается, что Borland "задал дурной тон", а другие его - банально подхватили. Без критической оценки.

И такого - "дурно пахнущего кода" - сотни строк у разных независимых разработчиков. Очень жаль, что драйвер процесса задал этот "дурной тон". Ну или я чего-то не понимаю.

Update. И беда как раз не в том, что "Borland сделал криво", в конце-концов - у них "всего пара классов", а в том, что сотни разработчиков эту кривизну подхватили. Ну или я чего-то недопонимаю.

Хотя не перестаю уважать разработчиков Delphi. Хорошее дело делают.

P.S. К коду FireMonkey - у меня тоже есть ряд претензий. Я их позже озвучу. Когда глубже вникну в проблему. Ну или напишу, что их нет, если пойму, что это так или убежу себя в том, что их нет.

P.P.S. Я объяснил свою позицию вроде. Буду рад, если кто-то объяснит её ошибочность.

P.P.P.S. Тут кстати явно в языке бы добавить "шаблонную типизацию с наследованием". Как-то так:

  TControlActionLink = class(TActionLink)
  template type TClientType = TControl;
  protected
    FClient: <TClientType>;
  private
    procedure AssignClient(aClient : <TClientType>);

...

  TWinControlActionLink = class(TControlActionLink)
  override template type TClientType = TWinControl;

....

С контролем всего этого хозяйства компилятором. У меня кстати подобное есть. В скриптовой машине. Правда там не статически, а в run-time это проверяется. Ну хотя бы в run-time - сейчас-то ведь не лучше. Сейчас по сути в run-time и происходит.

А ещё я бы TActionLink - сделал бы вложенным классом. Тоже с "шаблонной типизацией с наследованием". И различал бы abstract class и не abstract class. И требовал бы типизацию для неабстракных классов.

TControl = class(TCompenent)
 template type TControlActionLink = class(TActionLink)
  template type TClientType = TControl;
  protected
    FClient: <TClientType>;
  private
    procedure AssignClient(aClient : <TClientType>);
 ...
 end;
 ...
end;
 
 
TWinControl = class(TControl)
 reimplement template type TControlActionLink = class(TControl.TControlActionLink)
  override template type TClientType = TWinControl;
 end;
...
end;

-- как-то так. Сродни примесям где-то. Это можно кстати сделать через введение "скрытых виртуальных функций". То есть на уровне "препроцессора".

Ввести
function TClientType_Class: TControl; virtual;

Определять её в КАЖДОМ классе, где переопределён TClientType:


class function TControl.TControlActionLink.TClientType_Class: class of TControl; virtual;
begin
 Result := TControl;
end;
 
class function TWinControl.TControlActionLink.TClientType_Class: class of TControl; override;
begin
 Result := TWinControl;
end;
 
class function TControl.TControlActionLink_Class: class of TControl.TControlActionLink; virtual;
begin
 Result := TControl.TControlActionLink;
end;
 
class function TWinControl.TControlActionLink_Class: class of TControl.TControlActionLink; override;
begin
 Result := TWinControl.TControlActionLink;
end;



Вместо:
 FClient := aValue;
компилировать:
 FClient := aValue As TClientType_Class;

Вместо:
 FActionLink := TControlActionLink.Create(Self);
компилировать:
 FActionLink := ControlActionLink_Class.Create(Self);

Ну как-то так. Ну и по рекурсии конечно.

Ну и при статическом обращению к полю FClient и FActionLink - раскрывать их тип в зависимости уже от контекста. Для TControl подставлять одно, для TWinControl - другое. Всё кстати - срастается.


СЛОЖНО - да. Но не невозможно. У себя в скриптах - я пожалуй такое сделаю. Не вижу там препятствий.

Это конечно тоже - "Чудесный ужас". Но из другой оперы. Разрыв шаблонов.

В шаблонах кодогенерации я кстати уже сделал подобное. В шаблонах VCM (не путать с MVC, хотя это из той же оперы, только View->Controller->Model(->Data)). А я то и смотрю, что что-то - знакомое. Когда я до конца это расписал. Привести пример с картинками и кодом?

P.P.P.S. Подозреваю, что всё, что я описал - в каком-нибудь "экзотическом" языке программирования - УЖЕ есть. Мне вообще - пока не удалось - что-нибудь РЕАЛЬНО новое придумать.

понедельник, 13 мая 2013 г.

Странный код


protected void onPostExecute(SearchObject result) {
  result.ids = new HashSet<Long>();
  for (DocumentState doccc : result.docs) {
   result.ids.add(doccc.rowID);
  }
  responder.onFinishSearch(result);
 }