|
|
Регистрация | << Правила форума >> | FAQ | Пользователи | Календарь | Поиск | Сообщения за сегодня | Все разделы прочитаны |
|
Опции темы | Поиск в этой теме | Опции просмотра |
#1
|
|||
|
|||
Нужна не помощь, но оценка
В данный момент я переписываю свои шашки на чистовую, прочитал много информации о стандартах написания кода и попытался им соответствовать, но все еще сомневаюсь в его читабельности. Здесь должно быть много опытных программистов, которые могут указать на стилистические ошибки, так что я и решил выложить на ваш суд начальный этап работ, код главного меню, где изначально стоит анимированная заставка, после нажатия любой кнопки сменяется собственно меню, в котором 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
|
|||
|
|||
Такие вещи стоит приаттачивать архивом (чтобы экзешку глянуть, да и скомпилить ручками)
Код, если, честно - так себе. Его пока единственная заслуга - понятные имена переменных, плюс опущу оценку того факт, что они ещё с префиксами типов перменной (Венгерская нотация - достаточно спорная концепция, и сейчас уже от неё многие отвернулись). Код, в первую очередь, плох тем, что не следует принципу разделения логики и визуализации (интеракции с пользователем). В частности, это формируется тем, что нет никаких структур данных, вся логика зашита в обработчики событий на форме и контролы на ней. Также, хотя и особняком, стоит плохая декомпозиция (которая коррелирует с отсутствием структур данных), ведущая к большой копипасте кода. Немного связан с вышеназнываными проблемами и факт использования пачки глобальных переменных, во-первых, не имеющих никакого контроля над ними (как области видимости и доступа на чтение/запись, так и допутсимых значений), во-вторых, никак не структурированных. Плюс, чисто на любителя - я удаляю стандартные кэпские комменты вида { Public declarations } Как вижу правильную организаию данной задачи я: Есть сущности сцены и менеджера сцен (на данный момент сцены это сплеш-скрин и меню-скрин, а менеджер осуществляет их переключение, руководствуясь определённой логикой), есть сущности Контролов (на данный момент это кнопки, вроде как я вижу контрол с функцией "Фон", плюс возможно для курсора и прочих), и, конечно, сущность программного цикла, которая по сути некий таймер, рассылающий оповещения другим сущностям что произошёл квант времени и надо обновить свои состояния. Можно добавить и сущность ресурсного менеджера, который отвественен за ресурсы типа графика, шрифты, звуки - он грузит их, выгружает, выдаёт по требованию других сущностей. Поскольку у Вас много кода отведено кнопкам, можно рассмотреть их подробнее: Кнопка содержит унаследованные от сущности контрол свойства (позиция и размеры) и методы+события для реализации наведения курсора на кнопку, нажатия по кнопке и т.п. Специфичное для кнопки это состояния нажата, отжата, наведена и т.п. Состояние определяет и графические данные, при смене состояния меняются и данные, данные естественно инкапсулированы в сущность кнопки (например, в виде ссылок на конкретные граф.ресурсы из менеджера резурсов). |
Этот пользователь сказал Спасибо phomm за это полезное сообщение: | ||
dimosiko (09.07.2014)
|
#3
|
||||
|
||||
Повторов много, вот такое
Код:
... 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
|
|||
|
|||
Спасибо, буду работать над собой. Сильно волновали имена переменных и рад, что они оказались понятны, пора заняться структурой кода и некрасивыми повторениями.
|