Solved

Can this code improved ?

Posted on 2010-09-08
11
859 Views
Last Modified: 2013-12-13
Hello,

Following the discussions of july and august and want to start a new topic.
Prior related question:
http://www.experts-exchange.com/Q_26321089.html

I used the valuelisteditor for an application to add a location.
Going on and with help of experts i improved the code.

Two open issues:
1. The problem was that u must must use many booleans to make it work comform specifications.
2. I recognized that the setfocus method  not always select the field (make green on my computer)
    How can I make the error field selectable also when the cursor is on an other control ? (it happens  when   the focus is on button1)

Other comments are also welcome.

Here is code sofar ( I changed it a little )


procedure TForm1.ValueListEditor1SelectCell(Sender: TObject; ACol,
   ARow: Integer; var CanSelect: Boolean);
   begin
     if valid = false  then
     begin
       if ((ACol = ErrorCol) and (Arow = ErrorRow)) or (closing) then
          canselect:= true
       else
          canselect := false;

       button1.enabled:=false;
     end
     else
     begin
       CanSelect := true;
       button1.enabled:= true
     end;

     if closing then
        button1.enabled:=true
   end;



procedure TForm1.ValueListEditor1StringsChange(Sender: TObject);
begin
// check only as strings change and not close
   if not Closing
     then
       Checked:=False
     else
       Checked:=True;

end;


Function SetErrorCaption (ACol:Integer;ARow:Integer):string;
var Caption:String;
begin
   Form1.ValueListEditor1.Col:= ACol;
   Form11.ValueListEditor1.Row:= ARow;

   ErrorCol:= ACol;
   ErrorRow:= ARow;

   if Acol = 0 then
       Caption:= 'Locatie name not valid';

   if Acol = 1 then
       Caption:= 'number of places not valid';


   Agendaform1.Valuelisteditor1.Setfocus;

   result:= Caption;
end;

procedure TForm1.ValueListEditor1Validate(Sender: TObject; ACol, 
ARow: Integer;
const KeyName, KeyValue:string);

var KeyValueInt,PreviousValue:Integer; ErrorCaption:string;
begin

Try
if Checked= false then
begin

    Checked:=True;

    if trim (Keyname) = '' then
       begin
         ErrorCaption:= SetErrorCaption(0,Arow);
         raise Exception.create ('Locatiename has no valid value') 
       end;

     if tryStrtoInt (Trim(KeyValue),KeyValueInt) then
     begin
       if (KeyValueInt < 0) or (KeyvalueInt > 999)  then
         begin
           ErrorCaption:= SetErrorCaption(1,Arow);
           raise Exception.create('errormessage2');
         end
   
   
end;//checked
    // on exception show messagebox
    except
      on E : Exception do
       begin
         Windows.MessageBox(0,pWideChar(E.Message),PwideChar(ErrorCaption),mb_Ok);
         valid:=false;
       end;
    end; //except

end; //procedure

Open in new window

0
Comment
Question by:advance1
  • 5
  • 4
11 Comments
 
LVL 25

Accepted Solution

by:
epasquier earned 450 total points
ID: 33627147
1) If I remember your problem correctly, I think that your current code is about how tight as you can have it. You need those booleans to track some events combinations/order. So...

You could change a few details in coding style, but mostly there would be no change in the compiled results, only more readability of the code. I'm talking about details like
   If SomeBoolean = False Then
instead of
   If Not SomeBoolean Then

or the use of Windows.MessageBox instead of ShowMessage Delphi function.

or the fact that you can use Result as any variable, you don't need to set a value to a local variable and at the end set the result:=ThatVariable. Use directly Result.

and that :
   if not Closing
     then
       Checked:=False
     else
       Checked:=True;
can be replaced by :
  Checked:=Closing; // Much simpler no ?

2) I'm not sure what you are talking about with "I recognized that the setfocus method  not always select the field (make green on my computer)"
- If you mean that what you need is that the grid get the focus, that is what SetFocus is for.
- if you want also that a certain cell is selected, and so the focus shows a frame around the cell, that is what setting row/col is for
- and last, if by "make green" you mean that the text of this cell is selected in edit mode, the best you can do is to set the EditorMode property to True after the SetFocus

Why do you have Form1 and AgendaForm1 in your function ? is it a typo or do you really have 2 forms ? If you have only one, then you should consider making this function a method of that form, that would remove the need of specifying the form and use directly the ValueListEditor control.

If you really have 2 forms, please explain...
Function SetErrorCaption (ACol:Integer;ARow:Integer):string;

begin

// Form1.ValueListEditor1.Col:= ACol;

// Form1.ValueListEditor1.Row:= ARow;



 ErrorCol:= ACol;

 ErrorRow:= ARow;



 if Acol = 0 then result:= 'Locatie name not valid';

 if Acol = 1 then result:= 'number of places not valid';



 With Agendaform1.Valuelisteditor1 do

  begin

   Col:= ACol;

   Row:= ARow;

   Setfocus;

   EditorMode:=True;

  end;

end;

Open in new window

0
 
LVL 25

Expert Comment

by:epasquier
ID: 33628445
thanks for the move of the code, it was almost unreadable before
0
 

Author Comment

by:advance1
ID: 33629681

Thks for your comments  Good that you noticed,
I changed AgendaForm1 to Form1. So There is only 1 form (Form1).

I will check your changes.
0
 
LVL 32

Assisted Solution

by:ewangoya
ewangoya earned 50 total points
ID: 33633166
Just a few things I can think of on top of what epasquire has already noted

procedure TForm1.ValueListEditor1SelectCell(Sender: TObject; ACol,
   ARow: Integer; var CanSelect: Boolean);
begin
     if valid = false  then
     begin
         canselect := closing or ((ACol = ErrorCol) and (Arow = ErrorRow)) ;
         button1.enabled:=false;
     end
     else
      ........  
 end;

since you only execute these statements if Checked is false, you can put your try after the check
procedure TForm1.ValueListEditor1Validate(Sender: TObject; ACol, ARow: Integer;
  const KeyName, KeyValue:string);
var
  KeyValueInt, PreviousValue: Integer;
  ErrorCaption:string;
begin
  if Checked = false then
    try
      Checked := True;

      if Trim (Keyname) = '' then
      begin
        ErrorCaption:= SetErrorCaption(0,Arow);
        raise Exception.create ('Locatiename has no valid value')
      end;

      if tryStrtoInt (Trim(KeyValue), KeyValueInt) then
      begin
        if (KeyValueInt < 0) or (KeyvalueInt > 999)  then
         begin
           ErrorCaption:= SetErrorCaption(1,Arow);
           raise Exception.create('errormessage2');
         end
      end;//checked
      // on exception show messagebox
    except
      on E : Exception do
       begin
         Windows.MessageBox(0,pWideChar(E.Message),PwideChar(ErrorCaption),mb_Ok);
         valid:=false;
       end;
    end; //except
end; //procedure

You can also have
 case ACol of
    0: Result := 'Locatie name not valid';
    1: Result := 'number of places not valid';
  end;
0
 

Author Comment

by:advance1
ID: 33639188
Thks for the comments.
I think the comments are correct.
Its the old discussion is more cryptic code better code. I dont think this always so,

About the selectmode  I want the text of this cell is selected in edit mode,
I set the EditorMode property to True after the SetFocus, but when I clicked on button1 the error field is still not selected (This happens when I click the mouse).


0
Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 
LVL 25

Expert Comment

by:epasquier
ID: 33639281
you mean it's in edit mode, but not selected ?

I'm not sure that can be done (easily). I'll check that tomorrow if you confirm that is what you want  
0
 

Author Comment

by:advance1
ID: 33639389
Yes, I mean that.
Thks in advance !
0
 
LVL 25

Assisted Solution

by:epasquier
epasquier earned 450 total points
ID: 33645194
Well, as I said it's not 'doable' by conventional weapons. But a mini-nuke might help to get it done quite quickly.
The idea is to use the InplaceEditor property of any custom grid (from which TValueListEditor is derived), but it is a protected property, so unatainable.
What we have to do is make that property available, and that is done by redefining it
type
// redefinition of the TValueListEditor class
  TValueListEditor = class(ValEdit.TValueListEditor)
  public
    property InplaceEditor; // change this property to public
  end;
  TForm1 = class(TForm)
// ... rest of your form definition


//in the code, now you can access the InplaceEditor property
 With Agendaform1.Valuelisteditor1 do
  begin
   Col:= ACol;
   Row:= ARow;
   Setfocus;
   EditorMode:=True;
   InplaceEditor.SelectAll; // hehe!
  end;

Open in new window

0
 

Author Comment

by:advance1
ID: 33649466
Sounds good , I will try it Monday
0
 

Author Comment

by:advance1
ID: 33665265
Yess its ok !!

A lot of code but it works !

Thks.
I keep de question few days  open for other reactions,
0

Featured Post

What Security Threats Are You Missing?

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

This article covers general Notes 8.5 troubleshooting information including recreating the Notes\Data folder.
The article will include the best Data Recovery Tools along with their Features, Capabilities, and their Download Links. Hope you’ll enjoy it and will choose the one as required by you.
The viewer will learn how to set up a document for the web and print and the recommended PPI for printing.
The viewer will learn common shortcuts with easy ways to remember them. The viewer will then learn where to find all of the keyboard shortcuts, how to create/change them, and how to speed up their workflow.

746 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

9 Experts available now in Live!

Get 1:1 Help Now