3
\$\begingroup\$

I hope someone can guide me in a way to make this code more efficient. Please explain the things you do simply because I'm no pro so more complex things I am unlikely to get.

If you have issues with variable names or are unsure of functions of the code please let me know! I will clarify and help in any way I can.

program Noughts_And_Crosses_Bot_Simplified;

uses
  System.SysUtils, Math;

var
  GridPrint: array [1 .. 10] of string;   {Prints the correct symbol into the grid}
  GridStore: array [1 .. 10] of integer; {Stores a value of 1 or 2 in the grid depending on which turn it is}   
  WinCon: array [1 .. 10] of integer; {This adds up GridStore in all the rows and sees if any of them add up to a win}
  P1Bot, Counter, X, Co_Ords, Num, PlayerN, ChoiceA, y, z: integer;
  Symbol, ChoiceB, Co_OrdsB: string;
  Draw, Win, PlayerOnesGo: boolean;
  Player: String; {I'm hoping all the other variables should be self explanatory as you go through}

label
  Cont, Finish, Retry;
{This is the coding for when the bot has its go}     
Procedure PlayerOneBot;
label
  Retry, Finish;
begin
  for Counter := 1 to 3 do
  begin
    sleep(500);
    Write('.');
  end;
  writeln('');
  if WinCon[1] = ((2 * z) - 3) then {This is effectivly saying 'If Bot is about to win, make sure it wins. For clarification on variables X,Y and Z go to the bottom or leave a comment.}
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[2] = -3 then
    begin
      GridStore[2] := P1Bot;
      GridPrint[2] := Symbol;
      goto Finish;
    end;
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[2] = ((2 * z) - 3) then
  begin
    if GridStore[4] = -3 then
    begin
      GridStore[4] := P1Bot;
      GridPrint[4] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[6] = -3 then
    begin
      GridStore[6] := P1Bot;
      GridPrint[6] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[3] = ((2 * z) - 3) then
  begin
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
    if GridStore[8] = -3 then
    begin
      GridStore[8] := P1Bot;
      GridPrint[8] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[4] = ((2 * z) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[4] = -3 then
    begin
      GridStore[4] := P1Bot;
      GridPrint[4] := Symbol;
      goto Finish;
    end;
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[5] = ((2 * z) - 3) then
  begin
    if GridStore[2] = -3 then
    begin
      GridStore[2] := P1Bot;
      GridPrint[2] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[8] = -3 then
    begin
      GridStore[8] := P1Bot;
      GridPrint[8] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[6] = ((2 * z) - 3) then
  begin
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
    if GridStore[6] = -3 then
    begin
      GridStore[6] := P1Bot;
      GridPrint[6] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[7] = ((2 * z) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[8] = ((2 * z) - 3) then
  begin
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
  end;

  if WinCon[1] = ((2 * y) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[2] = -3 then
    begin
      GridStore[2] := P1Bot;
      GridPrint[2] := Symbol;
      goto Finish;
    end;
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[2] = ((2 * y) - 3) then
  begin
    if GridStore[4] = -3 then
    begin
      GridStore[4] := P1Bot;
      GridPrint[4] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[6] = -3 then
    begin
      GridStore[6] := P1Bot;
      GridPrint[6] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[3] = ((2 * y) - 3) then
  begin
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
    if GridStore[8] = -3 then
    begin
      GridStore[8] := P1Bot;
      GridPrint[8] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[4] = ((2 * y) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[4] = -3 then
    begin
      GridStore[4] := P1Bot;
      GridPrint[4] := Symbol;
      goto Finish;
    end;
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[5] = ((2 * y) - 3) then
  begin
    if GridStore[2] = -3 then
    begin
      GridStore[2] := P1Bot;
      GridPrint[2] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[8] = -3 then
    begin
      GridStore[8] := P1Bot;
      GridPrint[8] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[6] = ((2 * y) - 3) then
  begin
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
    if GridStore[6] = -3 then
    begin
      GridStore[6] := P1Bot;
      GridPrint[6] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[7] = ((2 * y) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[9] = -3 then
    begin
      GridStore[9] := P1Bot;
      GridPrint[9] := Symbol;
      goto Finish;
    end;
  end;
  if WinCon[8] = ((2 * y) - 3) then
  begin
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
    if GridStore[5] = -3 then
    begin
      GridStore[5] := P1Bot;
      GridPrint[5] := Symbol;
      goto Finish;
    end;
    if GridStore[7] = -3 then
    begin
      GridStore[7] := P1Bot;
      GridPrint[7] := Symbol;
      goto Finish;
    end;
  end;
Retry:
  Randomize;
  X := RandomRange(1, 10);
  if GridStore[X] <> -3 then
    goto Retry;
  GridStore[X] := P1Bot;
  GridPrint[X] := Symbol;
Finish:
end;

procedure Array_Set_Up;
var
  ArrayCounter: integer;
Begin
  for ArrayCounter := 1 to 9 do
  begin
    GridStore[ArrayCounter] := -3;
  end;
  for ArrayCounter := 1 to 10 do
    GridPrint[ArrayCounter] := ' ';
end;

Procedure Draw_Check;
label
finish;
Begin
  WinCon[1] := (GridStore[1]) + (GridStore[2]) + (GridStore[3]);
  WinCon[2] := (GridStore[4]) + (GridStore[5]) + (GridStore[6]);
  WinCon[3] := (GridStore[7]) + (GridStore[8]) + (GridStore[9]);
  WinCon[4] := (GridStore[1]) + (GridStore[4]) + (GridStore[7]);
  WinCon[5] := (GridStore[2]) + (GridStore[5]) + (GridStore[8]);
  WinCon[6] := (GridStore[3]) + (GridStore[6]) + (GridStore[9]);
  WinCon[7] := (GridStore[1]) + (GridStore[5]) + (GridStore[9]);
  WinCon[8] := (GridStore[3]) + (GridStore[5]) + (GridStore[7]);
  WinCon[9] := GridStore[1] + GridStore[2] + GridStore[3] + GridStore[4] +
    GridStore[5] + GridStore[6] + GridStore[7] + GridStore[8] + GridStore[9];
  if (WinCon[1] = 3) or (WinCon[2] = 3) or (WinCon[3] = 3) or (WinCon[4] = 3) or
    (WinCon[5] = 3) or (WinCon[6] = 3) or (WinCon[7] = 3) or (WinCon[8] = 3)
  then
  begin
    writeln('Player 1 Wins!');
    Win := True;
    goto Finish;
  end;
  if (WinCon[1] = 6) or (WinCon[2] = 6) or (WinCon[3] = 6) or (WinCon[4] = 6) or
    (WinCon[5] = 6) or (WinCon[6] = 6) or (WinCon[7] = 6) or (WinCon[8] = 6)
  then
  begin
    writeln('Player 2 Wins!');
    Win := True;
    goto Finish;
  end;
  if (WinCon[9] = 13) or (WinCon[9] = 14) then
  begin
    writeln('Game was a draw');
    Draw := True;
    goto Finish;
  end;
  Finish:
End;          {The grid has 9 values in it all set to -3 at the start, if player 1 where to go in say, top left that value would change to 1 so then WinCon[1] would be 1+-3+-3 so -5 however if Player 1 goes in all the top row it would be 1+1+1 so WinCon[1] would be 1 so player 1 would win}

Procedure Grid_Check;
label Retry, BotStop;
Begin
  Draw_Check;
  repeat
    if PlayerOnesGo = True then
    begin
      PlayerOnesGo := False;
      Player := 'Two';
      PlayerN := 2
    end
    Else
    begin
      PlayerOnesGo := True;
      Player := 'One';
      PlayerN := 1
    end;
    writeln('Player ' + Player + ' Go');
    if (PlayerOnesGo = True) and (P1Bot = 1) then
    begin
      PlayerOneBot;
      Goto BotStop;
    end;
    if (PlayerOnesGo = False) and (P1Bot = 2) then
    begin
      PlayerOneBot;
      Goto BotStop;
    end;
    writeln('Please enter the co-ords of your placemt');
    repeat
    Retry:
      readln(Co_OrdsB);
      if trystrtoint(Co_OrdsB, Num) then
      begin
        Co_Ords := Strtoint(Co_OrdsB);
        if (GridStore[Co_Ords] = 1) or (GridStore[Co_Ords] = 2) then
          writeln('This location is taken, Please pick another');
      end
      else
      begin
        writeln('Please input a number');
        goto Retry;
      end;
    until (GridStore[Co_Ords] = -3);

    GridStore[Co_Ords] := PlayerN;
    if PlayerOnesGo = True then
    begin
      GridPrint[Co_Ords] := 'X';
      GridStore[Co_Ords] := 1;
    end
    else
    begin
      GridPrint[Co_Ords] := 'O';
      GridStore[Co_Ords] := 2;
    end;
  BotStop:
    writeln('   ', GridPrint[1], ' | ', GridPrint[2], ' | ',
      GridPrint[3], '  ');
    writeln('   ----------');
    writeln('   ', GridPrint[4], ' | ', GridPrint[5], ' | ',
      GridPrint[6], '  ');
    writeln('   ----------');
    writeln('   ', GridPrint[7], ' | ', GridPrint[8], ' | ',
      GridPrint[9], '  ');
    Draw_Check;
  until (Win = True) or (Draw = True);
end;

begin
  Array_Set_Up;
Retry:
  writeln('Do you want to (1) Play against a bot or (2) Play against a friend?');
  readln(ChoiceB);
  if trystrtoint(ChoiceB, Num) then
  begin
    ChoiceA := Strtoint(ChoiceB);
    if ChoiceA = 2 then
      goto Finish
    else if ChoiceA = 1 then
      goto Cont
    else
    begin
      writeln('Please enter either 1 or 2');
      Goto Retry;
    end;
  end
  else
  begin
    writeln('Please enter a number');
    goto Retry;
  end;
Cont:
  Randomize;
  X := RandomRange(1, 3); {Says if bot is P1 or P2}
  if X = 1 then
  begin
    P1Bot := 1;
    Symbol := 'X';
    y := 2;  {Y Is used so it can see if the opponent is near winning}
    z := 1;  {Z is what it will put in gridstore when it picks a spot to go in}
  end;
  if X = 2 then
  begin
    P1Bot := 2;
    Symbol := 'O';
    y := 1;
    z := 1;
  end;
Finish:
  writeln('Noughts and crosses.');
  writeln('Game Board:');
  writeln('   ', GridPrint[1], ' | ', GridPrint[2], ' | ', GridPrint[3], '  ');
  writeln('   ----------');
  writeln('   ', GridPrint[4], ' | ', GridPrint[5], ' | ', GridPrint[6], '  ');
  writeln('   ----------');
  writeln('   ', GridPrint[7], ' | ', GridPrint[8], ' | ', GridPrint[9], '  ');
  writeln('The positions work as such, enter the co-ordinates of your move');
  writeln('    1 | 2 | 3 ');
  writeln('   -----------');
  writeln('    4 | 5 | 6 ');
  writeln('   -----------');
  writeln('    7 | 8 | 9 ');
  writeln('Game Start');
  PlayerOnesGo := False;
  Grid_Check;
  readln;

end.
\$\endgroup\$
4
\$\begingroup\$

I find it a little cocky that you understand that people might have issues with non-meaning variable names and undocumented functions, but are not prepared to put down a little work to remove those deficiencies.

Anyway, after 24 hours waiting for some clarifications in vain, first two corrections to your code:

  1. Randomize; is supposed to be called only once. Therefore move it to the beginning of the main program (between begin and the call to Array_Set_Up). Remove all other calls to Randomize.

  2. set the value of z := 2; instead of 1 when X = 2 in the setup of the bot. How about renaming it to what it represents, Opponent?

The code in procedure PlayerOneBot has a lot of code blocks that are equal, except for some indexes. There are sixteen of these blocks with variations in WinCon[], GridStore[] and GridPrint[] indexes and the z variable being replaced with the y variable in eight blocks:

  if WinCon[1] = ((2 * z) - 3) then
  begin
    if GridStore[1] = -3 then
    begin
      GridStore[1] := P1Bot;
      GridPrint[1] := Symbol;
      goto Finish;
    end;
    if GridStore[2] = -3 then
    begin
      GridStore[2] := P1Bot;
      GridPrint[2] := Symbol;
      goto Finish;
    end;
    if GridStore[3] = -3 then
    begin
      GridStore[3] := P1Bot;
      GridPrint[3] := Symbol;
      goto Finish;
    end;
  end;

To replace those code blocks, we want to create a function with appropriate arguments and return type, that we can call in a loop and perform the goto Finish jump if a cell has been selected.

The purpose of the first 8 blocks is to find an opportunity for the bot to win the game, by looking for a cell group (row, column or diagonal) with already two cells selected (by the bot) and the third unselected, and then selecting the third one.

If such an opportunity was not found, the following 8 blocks look for a cell group with two cells selected by the opponent and the third cell unselected. The third cell would then be selected by the bot, in order to prevent the opponent to select it at their next turn.

If none of the 16 blocks were successful, the bot selects a random unselected cell.

From descriptions above we can see that the concept of a CellGroup is important, but the definition is missing, so let's define some types and declare a const:

type
  TCellGroup = array[1..3] of integer;     // Holds the indexes of a cell group
  TCellGroups = array[1..8] of TCellGroup; // Holds the indexes of all cell groups

We can define a constant, holding all cell groups and their indexes (of the individual cells) for easy access:

const
  CellIndexes: TCellGroups =
    ((1, 2, 3), // rows
     (4, 5, 6),
     (7, 8, 9),
     (1, 4, 7), // cols
     (2, 5, 8),
     (3, 6, 9),
     (1, 5, 9), // diags
     (3, 5, 7));

Now we have the building blocks to declare a function that investigates a group for occurance of two selected (by the same player) cells and one unselected cell. Let's call that function CheckWinCon and give it two parameters, CellGroup and PlayerId

function CheckWinCon(CellGroup: integer; PlayerId: integer): boolean;
var
  n: integer;
begin
  Result := False;
  // If the player has marked 2 cells in a cellgroup and
  // if one cell in the cellgroup is empty (-3)
  if WinCon[CellGroup] = ((2 * PlayerId) - 3) then
  begin
    // find the empty cell
    for n := 1 to 3 do
    begin
      if GridStore[CellIndexes[CellGroup, n]] = -3 then
      begin
        // and select it
        GridStore[CellIndexes[CellGroup, n]] := P1Bot;
        GridPrint[CellIndexes[CellGroup, n]] := Symbol;
        Result := True;
        Exit;
      end;
    end;
  end;
end;

Declare an integer variable called CellGroup and place the function as a subfunction within procedure PlayerOneBot. Then replace the 16 if WinCon[]... blocks with the following two calls to the new function:

// check for an opportunity for the bot to win, and use it:
// if a cell group already has two cells selected by the bot
// and the third cell is free, then select the free cell to win
for CellGroup := 1 to 8 do
  if CheckWinCon(CellGroup, P1Bot) then goto Finish;

// check if opponent is about to win, and prevent it:
// if a cell group already has two cells selected by the opponent
// and the third cell is free, then select the free cell to prevent
// the opponent from selecting it on their turn
for CellGroup := 1 to 8 do
  if CheckWinCon(CellGroup, z) then goto Finish;
\$\endgroup\$
  • \$\begingroup\$ Firstly, sorry if I appeared cocky, that was not my intention, also I have been busy the last few days so sorry for not being able to clarify, as for the answer, this is really helpful and has absolutely reminded me of the need to go over functions. Thanks! \$\endgroup\$ – Geeekoid Apr 2 '18 at 15:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.