Hi all,
знаю, что за следующий код нужно голову отрывать, но не знаю как написать
по-другому ;(
Научите дурака
procedure TQForm.SaveToDo;
var
Tsk: TTask;
begin
if Edit1.Text <> ''then begin
Tsk := TTask.Create;
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit2.Text <> ''then begin
Tsk := TTask.Create;
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit3.Text <> ''then begin
Tsk := TTask.Create;
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit4.Text <> ''then begin
Tsk := TTask.Create;
if not AddTodoTask(Tsk) then Tsk.Free;
end;
end;
Здравствуйте, CR-LF, Вы писали:
CL>Hi all, CL>знаю, что за следующий код нужно голову отрывать, но не знаю как написать CL>по-другому ;(
...
сделать массив указателей на Edit'ы и проверять в цикле.
var
i: integer;
begin
for i:=0 to ComponentCount-1 do
if Components[i] is TEdit then
if (Components[i] as TEdit).Text <> ''then
AddTodoTask(TTask.Create);
end;
> По-моему, здесь для улучшения читаемости напрашивается переделка логики. > Один вопрос — какое >существует отношение между TTask и текстом в TEdit?
Дык с читаемостью-то тут как раз все хорошо в отличие от предложенных выше
массива указателей и Components[i] as TEdit.
А отношение самое что ни на есть прямое — TEdit.Text есть тот текст, который
описывает суть TTask
Здравствуйте, CR-LF, Вы писали:
CL>Hi all, CL>знаю, что за следующий код нужно голову отрывать, но не знаю как написать CL>по-другому ;( CL>Научите дурака CL>
[SKIP]
CL>
procedure TQForm.SaveToDo;
var Tsk : TTask;
begin
if (Edit1.Text <> '') OR (Edit2.Text <> '') OR (Edit3.Text <> '') OR (Edit4.Text <> '') then
begin
Tsk := TTask.Create;
if not AddTodoTask(Tsk) then Tsk.Free;
end;
end;
> procedure TQForm.SaveToDo;
> var Tsk : TTask;
> begin
> if (Edit1.Text <> '') OR (Edit2.Text <> '') OR (Edit3.Text <> '') OR
> (Edit4.Text <> '') then
> begin
> Tsk := TTask.Create;
> if not AddTodoTask(Tsk) then Tsk.Free;
> end;
> end;
>
Прошу прощения — я немного сократил оригинальный текст, а в результате
получилась глупость.
Более полный (но все еще не совсем оригинальный) вариант выглядел так
procedure TQForm.SaveToDo;
var
Tsk: TTask;
begin
if Edit1.Text <> ''then begin
Tsk := TTask.Create(Edit1.Text);
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit2.Text <> ''then begin
Tsk := TTask.Create(Edit2.Text);
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit3.Text <> ''then begin
Tsk := TTask.Create(Edit3.Text);
if not AddTodoTask(Tsk) then Tsk.Free;
end;
if Edit4.Text <> ''then begin
Tsk := TTask.Create(Edit4.Text);
if not AddTodoTask(Tsk) then Tsk.Free;
end;
end;
Здравствуйте, CR-LF, Вы писали:
CL>Прошу прощения — я немного сократил оригинальный текст, а в результате CL>получилась глупость. CL>Более полный (но все еще не совсем оригинальный) вариант выглядел так CL>
CL>procedure TQForm.SaveToDo;
CL>var
CL> Tsk: TTask;
CL>begin
CL> if Edit1.Text <> ''then begin
CL> Tsk := TTask.Create(Edit1.Text);
CL> if not AddTodoTask(Tsk) then Tsk.Free;
CL> end;
CL> if Edit2.Text <> ''then begin
CL> Tsk := TTask.Create(Edit2.Text);
CL> if not AddTodoTask(Tsk) then Tsk.Free;
CL> end;
CL> if Edit3.Text <> ''then begin
CL> Tsk := TTask.Create(Edit3.Text);
CL> if not AddTodoTask(Tsk) then Tsk.Free;
CL> end;
CL> if Edit4.Text <> ''then begin
CL> Tsk := TTask.Create(Edit4.Text);
CL> if not AddTodoTask(Tsk) then Tsk.Free;
CL> end;
CL>end;
CL>
Так это уже другое дело
Как-минимум, напрашивается перемещение дублирующегося кода в отдельный метод, например, все проверки выполнять в процедуре AddTodoTask:
procedure AddToDoTask (AText: string);
var
ATask: TTask;
begin
if AText = ''then
Exit;
// тут сделать еще проверки предусловий, при необходимости
// ....
// создаем экземпляр задания
ATask := TTask.Create (AText);
// тут выполнить добавление в задания
// ....end;
procedure TQForm.SaveToDo;
procedure SaveEditText(Edit: TCustomEdit);
var
Tsk: TTask;
begin
if Edit.Text <> ''then begin
Tsk := TTask.Create(Edit.Text);
if not AddTodoTask(Tsk) then Tsk.Free;
end;
end;
begin
SaveEditText(Edit1);
SaveEditText(Edit2);
SaveEditText(Edit3);
SaveEditText(Edit4);
SaveEditText(Edit5);
end;
А что есть в твоем AddTodoTask, что нужно проверять результат функции на False? Может, вынести эту проверку "наверх"? Иначе как-то оно кривовато: сначала создать екземпляр объекта, а затем удалять его при невыполнении какого-то условия. Не лучше ли будет сразу НЕ создавать его при определенном условии?
>А что есть в твоем AddTodoTask, что нужно проверять результат функции на >False? Может, вынести эту >проверку "наверх"? Иначе как-то оно кривовато: сначала создать екземпляр >объекта, а затем удалять его при >невыполнении какого-то условия. Не лучше ли будет сразу НЕ создавать его >при определенном условии?
Ну это условие выясняется внутри AddTodoTask.
В общем долго рассказывать.
А че такого ? — Если AddTodoTask сработала, значит объект используется и
надо его оставить, а если нет — надо его удалить.
Здравствуйте, CR-LF, Вы писали:
>>А что есть в твоем AddTodoTask, что нужно проверять результат функции на >>False? Может, вынести эту >>проверку "наверх"? Иначе как-то оно кривовато: сначала создать екземпляр >>объекта, а затем удалять его при >>невыполнении какого-то условия. Не лучше ли будет сразу НЕ создавать его >>при определенном условии? CL>Ну это условие выясняется внутри AddTodoTask. CL>В общем долго рассказывать. CL>А че такого ? — Если AddTodoTask сработала, значит объект используется и CL>надо его оставить, а если нет — надо его удалить
Да просто мне кажется, что можно еще немного улучшить код. А можно увидеть хотя бы псевдокод этого AddTodoTask? Страсть как люблю поломать голову над такими вещами
> Да просто мне кажется, что можно еще немного улучшить код. А можно увидеть > хотя бы псевдокод этого >AddTodoTask? Страсть как люблю поломать голову над такими вещами
Чудак-человек
Ну на смотри, сильно только не ругай.
function TQForm.AddTodoTask(Tsk: TTask): Boolean;
var
i: Integer;
Msg: String;
begin
Result := False;
i := TaskAlreadyExists(Tsk);
if i <> -1 then begin
if Task(i).StartTime = 0 then
Msg := 'Задача с таким описанием уже существует'else
Msg := FormatDateTime('Задача с таким описанием уже
существует,'#13'назначена на dd.mm.yy hh:mm',
Task(i).StartTime);
if MessageDlg(Msg + #13'Переназначить ?',
mtConfirmation, [mbYes,mbNo],0) <> mrYes
then
Exit
else
Tasks.Delete(i);
end;
Tasks.Add(Tsk);
Tasks.Sort(@CompareTasks);
UpdateTodoListBox;
Result := True;
end;
Ну, я так и знал!
Тут несколько замечаний, основные:
1. Обработку ошибок не лучше ли сделать на основе исключений?
2. В приведенном выше коде мешанина логики с GUI, не есть это гуд
Впрочем, если тебе это слушать неприятно и код тебя устраивает — мое дело сторона
> Ну, я так и знал! > Тут несколько замечаний, основные: > 1. Обработку ошибок не лучше ли сделать на основе исключений?
Я че-то не понимаю каким образом воткнуть туда исключения.
> 2. В приведенном выше коде мешанина логики с GUI, не есть это гуд
Пожалуй что так, но как иначе-то ?
> Впрочем, если тебе это слушать неприятно и код тебя устраивает — мое дело > сторона
Ну почему же неприятно — наоборот.
Просто в институте никогда и ни один из преподов такой критикой не
утруждался.
Я, конечно, догадывался, что здесь не все чисто, но ведь человека не
заставишь сказать правду, если он этого не хочет
Не понял, где тут частные сообщения ? ;(
Короче, конечно не возражаю.
Могу даже всю программу прислать, если хочешь )
Там еще 1700 строк такого мусора.
Правда в указанное тобой время я уже буду спать — у нас уже 2.40, но с утра
с удовольствием погляжу.
unit Tasks;
interface
uses SysUtils, Classes, Contnrs;
type
ETaskManagerError = class (Exception);
TTask = class
private
FName: string;
FStartTime: TDateTime;
protected
function HasStartTime: Boolean;
public
constructor Create (AName: string; AppendStartTime: Boolean = True);
property Name: string read FName;
property StartTime: TDateTime read FStartTime;
function ToString: string;
end;
TTaskManager = class
private
FList: TObjectList;
function GetCount: Integer;
function GetItems(Index: Integer): TTask;
protected
procedure Add (ATask: TTask);
public
function CreateTask (ATaskName: string; AppendStartTime: Boolean = True): TTask;
procedure RenameTask (ExistingIndex: Integer; NewName: string);
function IndexOf (ATaskName: string): Integer;
constructor Create;
destructor Destroy; override;
property Items[Index: Integer]: TTask read GetItems; default;
property Count: Integer read GetCount;
procedure Delete (Index: Integer);
end;
implementation{ TTask }constructor TTask.Create(AName: string;
AppendStartTime: Boolean = True);
begin
inherited Create();
FName := AName;
if AppendStartTime then
FStartTime := Now();
end;
function TTask.HasStartTime: Boolean;
begin
Result := FStartTime<>0
end;
function TTask.ToString: string;
begin
Result := FName;
if HasStartTime then
Result := Format ('%s (%s)',[Result, DateTimeToStr(FStartTime)]);
end;
{ TTaskManager }procedure TTaskManager.Add(ATask: TTask);
begin
FList.Add(ATask)
end;
constructor TTaskManager.Create;
begin
inherited Create();
FList := TObjectList.Create (True); // owns objectsend;
function TTaskManager.CreateTask(ATaskName: string;
AppendStartTime: Boolean = True): TTask;
begin// проверка пост-условия:
// в списке не должно уже быть задания с таким именемif IndexOf(ATaskName)<>-1 then
raise ETaskManagerError.Create('Задание с таким именем уже существует');
Result := TTask.Create(ATaskName, AppendStartTime);
Add (Result);
end;
procedure TTaskManager.Delete(Index: Integer);
begin
FList.Delete(Index);
end;
destructor TTaskManager.Destroy;
begin
FList.Free;
inherited;
end;
function TTaskManager.GetCount: Integer;
begin
Result := FList.Count
end;
function TTaskManager.GetItems(Index: Integer): TTask;
begin
Result := TTask(FList[Index])
end;
function TTaskManager.IndexOf(ATaskName: string): Integer;
var
I: Integer;
begin
Result := -1;
for I := 0 to FList.Count - 1 do
if AnsiCompareText(ATaskName,GetItems(I).Name)=0 then
begin
Result := I;
Break
end;
end;
procedure TTaskManager.RenameTask(ExistingIndex: Integer; NewName: string);
var
Tsk: TTask;
begin
Tsk := GetItems(ExistingIndex);
// возможно, это не лучший вариант с доступом к приватному полю,
// но я иногда так делаю
Tsk.FName := NewName;
end;
end.
Гуи:
unit Unit18;
interface
uses
Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
Dialogs, Tasks, StdCtrls;
type
TQForm = class(TForm)
lbToDo: TListBox;
btnSaveTasks: TButton;
Edit1: TEdit;
Edit2: TEdit;
Edit3: TEdit;
procedure btnSaveTasksClick(Sender: TObject);
procedure FormDestroy(Sender: TObject);
procedure FormCreate(Sender: TObject);
private
FTaskManager: TTaskManager;
FEditBoxes: array of TEdit;
function AddTask (AName: string): Boolean;
function AskForRename (Tsk: TTask): Boolean;
procedure UpdateToDoListBox;
end;
var
QForm: TQForm;
implementation{$R *.dfm}function TQForm.AddTask(AName: string): Boolean;
var
Idx: Integer;
begin
Result := False;
if Trim(AName)=''then
Exit;
Idx := FTaskManager.IndexOf(AName);
if Idx = -1 then
begin
FTaskManager.CreateTask(AName);
Result := True
end
else
if AskForRename(FTaskManager[Idx]) then
begin
FTaskManager.RenameTask(Idx,AName);
Result := True
end;
end;
function TQForm.AskForRename(Tsk: TTask): Boolean;
begin
Result :=
MessageDlg('Задача "' + Tsk.ToString + '" уже существует. Переназначить?',
mtWarning,[mbYes,mbNo],-1) = mrYes
end;
///
/// обработчик нажатия на кнопку "Сохранить задания..."
///procedure TQForm.btnSaveTasksClick(Sender: TObject);
var
I: Integer;
TasksChanged: Boolean;
begin
TasksChanged := False;
for I := Low(FEditBoxes) to High(FEditBoxes) do
if AddTask(FEditBoxes[I].Text) then
TasksChanged := True;
if TasksChanged then
begin//
// тут выполняем сортировку и т.п
// ...
// обновляем визуальный список
UpdateToDoListBox;
end;
end;
procedure TQForm.FormCreate(Sender: TObject);
begin
FTaskManager := TTaskManager.Create;
// Честно говоря, не знаю, зачем тебе несколько
// эдитов, поэтому сделал пока так.
// В зависимости от задачи (которая мне не известна)
// способ хранения и доступа к полям ввода может модифицироваться
SetLength(FEditBoxes,3);
FEditBoxes[0] := Edit1;
FEditBoxes[1] := Edit2;
FEditBoxes[2] := Edit3;
end;
procedure TQForm.FormDestroy(Sender: TObject);
begin
FTaskManager.Free;
end;
procedure TQForm.UpdateToDoListBox;
var
I: Integer;
begin
lbToDo.Items.BeginUpdate;
try
lbToDo.Clear;
for I := 0 to FTaskManager.Count-1 do
lbToDo.Items.Add(FTaskManager[I].ToString)
finally
lbToDo.Items.EndUpdate;
end;
end;
end.
Гуишную часть можно сделать и по-другому, но я не понял смысл этих нескольких эдитов и переспрашивания пользователя
Приношу извинения. Из-за недостатка времени в спешке "наваял" совсем не то, совсем заработался.
Если завтра будет актуально — исправлюсь, еще раз сорри.