help to fix my if then statments! or errors in my code :( 1

[Simon180]

I am having abit of problem with the code before I want it to check to see if a user is the mic handly and if there are it will allow them to remove mic if not then it would just exit or have a null effect!, the problem is when i try the code it removes the mic from only the mic handlyer side and no the rest of the clients.

can anyone help?

      if (com = 'DelMic') then
      begin
          for i := 0 to MainServer.Socket.ActiveConnections - 1 do
          begin
            if lowercase(MainServer.Socket.Connections[i].Room) = lowercase(dat[1]) then
            begin

            if dat[0] = RoomsTable.FieldByName('Talker').AsString then // check user with talker entry in database
            begin
               MainServer.Socket.Connections[i].SendText(CodeStr('šDelMicœ' // send remove chater command
               + dat[0] + 'œ'    // user name
               + dat[1] + 'œ')); // room name

            if lowercase(MainServer.Socket.Connections[i].Name) = lowercase(dat[0]) then
                         MainServer.Socket.Connections[i].Speak := False;
                         end;

            if RoomsTable.Locate('Name', dat[1], [locaseinsensitive]) then
            begin
               RoomsTable.Edit;
               RoomsTable.FieldByName('Talker').AsString := 'FREE'; // update database entry
               RoomsTable.Post;
            end
            else
             exit; // stop other clients from setting there mic when a other user is using mic!
          end;
        end;
      end;

thanks alot

 

[Griffyn]

First up, when do case-insensitive string comparisons, don't use lowercase or uppercase on both sides - simply use the SameText function.

I've had a good look at your code and realised that I still don't understand what you're trying to do. Perhaps try writing some pseudo-code to illustrate the logic your code is trying to achieve.

 

[AP81]

Thanks Griff.. never heard of the SameText function. will save me doing:

if uppercase(string1) = uppercase(string2)

 

[Glenn9999]

There's something to be said for code review - much can be done for human consumption. Ultimately, IMO, part of good coding is to make it understandable in a way that both you and someone else can come along and understand it - to that end, code that works on a computer doesn't necessarily work for the programmer as well. As a code reviewer, I would flunk this section of code for three reasons:

1) While you can do infinite sub-levels in logic, it's not wise to do so, so it can be easily understood. The rule that I always used for code is that if it's more than three sub-levels, then it needs rewritten so the inner logic is broken out into procedures/functions that perform discrete functions in the logic. By looks of this code, it could be broken out into at least three separate discrete procedures.

2) The formatting is off - while the compiler doesn't need the formatting, it's much easier on us if proper formatting is there, so what is going on may be easily understood. Readability is always key.

3) This section of code:

            if RoomsTable.Locate('Name', dat[1], [locaseinsensitive]) then
            begin
               RoomsTable.Edit;
               RoomsTable.FieldByName('Talker').AsString := 'FREE'; // update database entry
               RoomsTable.Post;
            end
            else
             exit; // stop other clients from setting there mic when a other user is using mic!

has nothing to do with the loop that it is within in the logic (the loop control variable is not referenced) and should be removed from the loop.

The lowercase() = lowercase() thing has already been referenced - I would say if the dat array is something that can be changed within your program, it would be good to standardize it when you load it (using lowercase there) instead of using it everytime you test against the table.

 

[Simon180]

ty for the help basic want i am trying to do is add a check to see if sumone is allready on the mic and if there are then it would reject any other request if there username dont match the username in the talker database field.