Форум по Delphi программированию

Delphi Sources



Вернуться   Форум по Delphi программированию > Все о Delphi > [ "Начинающим" ]
Ник
Пароль
Регистрация <<         Правила форума         >> FAQ Пользователи Календарь Поиск Сообщения за сегодня Все разделы прочитаны

Ответ
 
Опции темы Поиск в этой теме Опции просмотра
  #1  
Старый 08.07.2014, 18:58
dimosiko dimosiko вне форума
Прохожий
 
Регистрация: 08.07.2014
Сообщения: 2
Версия Delphi: Delphi 2007
Репутация: 10
Сообщение Нужна не помощь, но оценка

В данный момент я переписываю свои шашки на чистовую, прочитал много информации о стандартах написания кода и попытался им соответствовать, но все еще сомневаюсь в его читабельности. Здесь должно быть много опытных программистов, которые могут указать на стилистические ошибки, так что я и решил выложить на ваш суд начальный этап работ, код главного меню, где изначально стоит анимированная заставка, после нажатия любой кнопки сменяется собственно меню, в котором 4 кнопки, роль которых исполняют TImage, при наведении мыши на "кнопку" и клике также присутствует анимация. Главный вопрос: читабельно ли и соответствует ли стандартам?
(да, для кнопки "правила" (rules) вообще почти ничего нет в сравнении с другими)
Код:
unit MainMenu;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, ExtCtrls;

type
  TMainForm = class(TForm)
    imgBackground: TImage;
    imgButtonEditor: TImage;
    imgButtonNewGame: TImage;
    imgButtonRules: TImage;
    imgButtonExit: TImage;
    tmAnimation: TTimer;
    procedure FormCreate(Sender: TObject);
    procedure tmAnimationTimer(Sender: TObject);
    procedure FormKeyPress(Sender: TObject; var Key: Char);
    procedure ButtonMouseEnter(Sender: TObject);
    procedure ButtonClick(Sender: TObject);
    procedure ButtonMouseDown(Sender: TObject; Button: TMouseButton;
      Shift: TShiftState; X, Y: Integer);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  MainForm: TMainForm;
  btmpLightChecker, btmpDarkChecker, btmpLightKing, btmpDarkKing, btmpEaten,
  btmpLightDelete, btmpDarkDelete, btmpChosen, btmpCanBeChosen,
  btmpCanBeEaten, btmpCursorHere, btmpCanMoveHere: TBitmap;
  iAnimationNumber: integer;
  blEditorMouseEnter, blNewGameMouseEnter, blRulesMouseEnter, blExitMouseEnter,
  blHeadband: boolean;

implementation

{$R *.dfm}

procedure TMainForm.FormCreate(Sender: TObject);
begin
  DoubleBuffered := True;
  blHeadband := True;

  btmpLightChecker := Graphics.TBitmap.Create;
  btmpDarkChecker := Graphics.TBitmap.Create;
  btmpLightKing := Graphics.TBitmap.Create;
  btmpDarkKing := Graphics.TBitmap.Create;
  btmpLightDelete := Graphics.TBitmap.Create;
  btmpDarkDelete := Graphics.TBitmap.Create;
  btmpChosen := Graphics.TBitmap.Create;
  btmpCanBeChosen := Graphics.TBitmap.Create;
  btmpEaten := Graphics.TBitmap.Create;
  btmpCanBeEaten := Graphics.TBitmap.Create;
  btmpCursorHere := Graphics.TBitmap.Create;
  btmpCanMoveHere := Graphics.TBitmap.Create;

  btmpLightChecker.LoadFromFile('Images\Checkers\LightChecker.bmp');
  btmpDarkChecker.LoadFromFile('Images\Checkers\DarkChecker.bmp');
  btmpLightKing.LoadFromFile('Images\Checkers\LightKing.bmp');
  btmpDarkKing.LoadFromFile('Images\Checkers\DarkKing.bmp');

  btmpChosen.LoadFromFile('Images\Other\Chosen.bmp');
  btmpChosen.Transparent:= true;
  btmpChosen.transparentMode:= TmFixed;
  btmpChosen.TransparentColor:= RGB(0, 0, 0);

  btmpCanBeChosen.LoadFromFile('Images\Other\CanBeChosen.bmp');
  btmpCanBeChosen.Transparent:= true;
  btmpCanBeChosen.transparentMode:= TmFixed;
  btmpCanBeChosen.TransparentColor:= RGB(0, 0, 0);

  btmpEaten.LoadFromFile('Images\Other\AlreadyEaten.bmp');
  btmpEaten.Transparent:= true;
  btmpEaten.transparentMode:= TmFixed;
  btmpEaten.TransparentColor:= RGB(0, 0, 0);

  btmpCanBeEaten.LoadFromFile('Images\Other\CanBeEaten.bmp');
  btmpCanBeEaten.Transparent:= true;
  btmpCanBeEaten.transparentMode:= TmFixed;
  btmpCanBeEaten.TransparentColor:= RGB(0, 0, 0);

  btmpCursorHere.LoadFromFile('Images\Other\CursorIsHere.bmp');
  btmpCursorHere.Transparent:= true;
  btmpCursorHere.transparentMode:= TmFixed;
  btmpCursorHere.TransparentColor:= RGB(0, 0, 0);

  btmpCanMoveHere.LoadFromFile('Images\Other\CanMoveHere.bmp');
  btmpCanMoveHere.Transparent:= true;
  btmpCanMoveHere.transparentMode:= TmFixed;
  btmpCanMoveHere.TransparentColor:= RGB(0, 0, 0);

  btmpLightDelete.LoadFromFile('Images\Other\DeleteChoiseFromLight.bmp');
  btmpLightDelete.Transparent:= true;
  btmpLightDelete.transparentMode:= TmFixed;
  btmpLightDelete.TransparentColor:= RGB(0, 0, 0);

  btmpDarkDelete.LoadFromFile('Images\Other\DeleteChoiseFromDark.bmp');
  btmpDarkDelete.Transparent:= true;
  btmpDarkDelete.transparentMode:= TmFixed;
  btmpDarkDelete.TransparentColor:= RGB(0, 0, 0);
end;

procedure TMainForm.FormKeyPress(Sender: TObject; var Key: Char);
begin
  blHeadband := False;
  tmAnimation.Interval := 100;
  imgBackground.Picture.LoadFromFile('Images\Backgrounds\MainMenu.bmp');
  imgButtonEditor.Visible := True;
  imgButtonNewGame.Visible := True;
  imgButtonRules.Visible := True;
  imgButtonExit.Visible := True;
end;

procedure TMainForm.ButtonMouseEnter(Sender: TObject);
begin
  if not blHeadband then
    iAnimationNumber := 1;
  if Sender = imgButtonEditor then
    blEditorMouseEnter := True;
  if Sender = imgButtonNewGame then
    blNewGameMouseEnter := True;
  if Sender = imgButtonRules then
    blRulesMouseEnter := True;
  if Sender = imgButtonExit then
    blExitMouseEnter := True;
  if (Sender = imgBackground) and not blHeadband then
    begin
      blEditorMouseEnter := False;
      blNewGameMouseEnter := False;
      blRulesMouseEnter := False;
      blExitMouseEnter := False;
      imgButtonEditor.Picture.LoadFromFile('Images\Animations\Buttons\Editor\Animation1.bmp');
      imgButtonNewGame.Picture.LoadFromFile('Images\Animations\Buttons\NewGame\Animation1.bmp');
      imgButtonRules.Picture.LoadFromFile('Images\Animations\Buttons\Rules\Animation1.bmp');
      imgButtonExit.Picture.LoadFromFile('Images\Animations\Buttons\Exit\Animation1.bmp');
    end;
end;

procedure TMainForm.ButtonMouseDown(Sender: TObject; Button: TMouseButton;
  Shift: TShiftState; X, Y: Integer);
begin
  if Sender = imgButtonEditor then
  begin
    imgButtonEditor.Picture.LoadFromFile('Images\Animations\Buttons\Editor\Animation5.bmp');
    blEditorMouseEnter := False;
  end;
  if Sender = imgButtonNewGame then
  begin
    imgButtonNewGame.Picture.LoadFromFile('Images\Animations\Buttons\NewGame\Animation5.bmp');
    blNewGameMouseEnter := False;
  end;
{  if Sender = imgButtonRules then
  begin

  end; }
  if Sender = imgButtonExit then
  begin
    imgButtonExit.Picture.LoadFromFile('Images\Animations\Buttons\Exit\Animation5.bmp');
    blExitMouseEnter := False;
  end;
end;

procedure TMainForm.ButtonClick(Sender: TObject);
begin
  if Sender = imgButtonEditor then
  begin
    imgButtonEditor.Picture.LoadFromFile('Images\Animations\Buttons\Editor\Animation1.bmp');
  end;
  if Sender = imgButtonNewGame then
  begin
    imgButtonNewGame.Picture.LoadFromFile('Images\Animations\Buttons\NewGame\Animation1.bmp');
  end;
{  if Sender = imgButtonRules then
  begin

  end;  }
  if Sender = imgButtonExit then
  begin
    imgButtonExit.Picture.LoadFromFile('Images\Animations\Buttons\Exit\Animation1.bmp');
    MainForm.Close;
  end;
end;

procedure TMainForm.tmAnimationTimer(Sender: TObject);
begin
  if blHeadband then
  begin
    iAnimationNumber := iAnimationNumber+1;
    imgBackground.Picture.LoadFromFile
    ('Images\Animations\headband\Animation'+inttostr(iAnimationNumber)+'.bmp');
    if iAnimationNumber = 37 then
      iAnimationNumber := 1;
  end;
  if blEditorMouseEnter then
  begin
    iAnimationNumber := iAnimationNumber+1;
    imgButtonEditor.Picture.LoadFromFile
    ('Images\Animations\Buttons\Editor\Animation'+inttostr(iAnimationNumber)+'.bmp');
    if iAnimationNumber = 4 then
      iAnimationNumber := 1;
  end;
  if blNewGameMouseEnter then
  begin
    iAnimationNumber := iAnimationNumber+1;
    imgButtonNewGame.Picture.LoadFromFile
    ('Images\Animations\Buttons\NewGame\Animation'+inttostr(iAnimationNumber)+'.bmp');
    if iAnimationNumber = 4 then
      iAnimationNumber := 1;
  end;
  if blRulesMouseEnter then
  begin
    iAnimationNumber := iAnimationNumber+1;
    imgButtonRules.Picture.LoadFromFile
    ('Images\Animations\Buttons\Rules\Animation'+inttostr(iAnimationNumber)+'.bmp');
    if iAnimationNumber = 4 then
      iAnimationNumber := 1;
  end;
  if blExitMouseEnter then
  begin
    iAnimationNumber := iAnimationNumber+1;
    imgButtonExit.Picture.LoadFromFile
    ('Images\Animations\Buttons\Exit\Animation'+inttostr(iAnimationNumber)+'.bmp');
    if iAnimationNumber = 4 then
      iAnimationNumber := 1;
  end;
end;

end.
Ответить с цитированием
  #2  
Старый 08.07.2014, 22:30
phomm phomm вне форума
Новичок
 
Регистрация: 07.10.2013
Адрес: Тюмень
Сообщения: 50
Версия Delphi: 7/2007/XE+/FPC
Репутация: 22
По умолчанию

Такие вещи стоит приаттачивать архивом (чтобы экзешку глянуть, да и скомпилить ручками)

Код, если, честно - так себе. Его пока единственная заслуга - понятные имена переменных, плюс опущу оценку того факт, что они ещё с префиксами типов перменной (Венгерская нотация - достаточно спорная концепция, и сейчас уже от неё многие отвернулись).

Код, в первую очередь, плох тем, что не следует принципу разделения логики и визуализации (интеракции с пользователем). В частности, это формируется тем, что нет никаких структур данных, вся логика зашита в обработчики событий на форме и контролы на ней.
Также, хотя и особняком, стоит плохая декомпозиция (которая коррелирует с отсутствием структур данных), ведущая к большой копипасте кода.
Немного связан с вышеназнываными проблемами и факт использования пачки глобальных переменных, во-первых, не имеющих никакого контроля над ними (как области видимости и доступа на чтение/запись, так и допутсимых значений), во-вторых, никак не структурированных.

Плюс, чисто на любителя - я удаляю стандартные кэпские комменты вида { Public declarations }

Как вижу правильную организаию данной задачи я: Есть сущности сцены и менеджера сцен (на данный момент сцены это сплеш-скрин и меню-скрин, а менеджер осуществляет их переключение, руководствуясь определённой логикой), есть сущности Контролов (на данный момент это кнопки, вроде как я вижу контрол с функцией "Фон", плюс возможно для курсора и прочих), и, конечно, сущность программного цикла, которая по сути некий таймер, рассылающий оповещения другим сущностям что произошёл квант времени и надо обновить свои состояния. Можно добавить и сущность ресурсного менеджера, который отвественен за ресурсы типа графика, шрифты, звуки - он грузит их, выгружает, выдаёт по требованию других сущностей.
Поскольку у Вас много кода отведено кнопкам, можно рассмотреть их подробнее: Кнопка содержит унаследованные от сущности контрол свойства (позиция и размеры) и методы+события для реализации наведения курсора на кнопку, нажатия по кнопке и т.п. Специфичное для кнопки это состояния нажата, отжата, наведена и т.п. Состояние определяет и графические данные, при смене состояния меняются и данные, данные естественно инкапсулированы в сущность кнопки (например, в виде ссылок на конкретные граф.ресурсы из менеджера резурсов).
Ответить с цитированием
Этот пользователь сказал Спасибо phomm за это полезное сообщение:
dimosiko (09.07.2014)
  #3  
Старый 09.07.2014, 05:46
Аватар для Alegun
Alegun Alegun вне форума
LMD-DML
 
Регистрация: 12.07.2009
Адрес: Богородское
Сообщения: 3,025
Версия Delphi: D7E
Репутация: 1834
По умолчанию

Повторов много, вот такое
Код:
...
 btmpChosen.LoadFromFile('Images\Other\Chosen.bmp');
 btmpChosen.Transparent:= true;
 btmpChosen.transparentMode:= TmFixed;
 btmpChosen.TransparentColor:= RGB(0, 0, 0);
...
можно подсократить вынеся в отдельную процедуру
Код:
procedure ds(img: string; var btmp: TBitmap);
begin
 with btmp do
  begin
   LoadFromFile(img);
   Transparent:= true;
   TransparentMode:= TmFixed;
   TransparentColor:= RGB(0, 0, 0);
  end;
end;
а вот и её вызовы
Код:
...
 ds('Images\Other\Chosen.bmp', btmpChosen);
 ds('Images\Other\CanBeChosen.bmp', btmpCanBeChosen);
 ds('Images\Other\AlreadyEaten.bmp', btmpEaten);
 ds('Images\Other\CanBeEaten.bmp', btmpCanBeEaten);
 ds('Images\Other\CursorIsHere.bmp', btmpCursorHere);
 ds('Images\Other\CanMoveHere.bmp', btmpCanMoveHere);
 ds('Images\Other\DeleteChoiseFromLight.bmp', btmpLightDelete);
 ds('Images\Other\DeleteChoiseFromDark.bmp', btmpDarkDelete);
...
Ответить с цитированием
Этот пользователь сказал Спасибо Alegun за это полезное сообщение:
dimosiko (09.07.2014)
  #4  
Старый 09.07.2014, 08:06
dimosiko dimosiko вне форума
Прохожий
 
Регистрация: 08.07.2014
Сообщения: 2
Версия Delphi: Delphi 2007
Репутация: 10
По умолчанию

Спасибо, буду работать над собой. Сильно волновали имена переменных и рад, что они оказались понятны, пора заняться структурой кода и некрасивыми повторениями.
Ответить с цитированием
Ответ


Delphi Sources

Опции темы Поиск в этой теме
Поиск в этой теме:

Расширенный поиск
Опции просмотра

Ваши права в разделе
Вы не можете создавать темы
Вы не можете отвечать на сообщения
Вы не можете прикреплять файлы
Вы не можете редактировать сообщения

BB-коды Вкл.
Смайлы Вкл.
[IMG] код Вкл.
HTML код Выкл.
Быстрый переход


Часовой пояс GMT +3, время: 00:33.


 

Сайт

Форум

FAQ

RSS лента

Прочее

 

Copyright © Форум "Delphi Sources" by BrokenByte Software, 2004-2023

ВКонтакте   Facebook   Twitter