Error with prime number checker.

[CodeWarrior14]

Hello everyone,
I'm new here - just joined.

I'm strugling with a bit of code. Im trying to check if a number is a prime number, but my program produces weird results.
Herse the code:

{ Vefies if a value is prime. }
function TMactacHash.IsPrime(value: integer): boolean;
var
  i, overValue: integer;
  flag: boolean;
begin
  flag := true;

  for i := 1 to value do
    begin

      overValue := value mod i;
      if overValue = 0 then
        flag := false;

      WriteLn(IntToStr(value) + ' mod ' + IntToStr(i) + ' = ' + IntToStr(overValue));
      readln;

    end;

  Result := flag;
end;

{ Generates the prime number. }
function TMactacHash.GeneratePrime(primeCounter: integer; _set: PrimeSets): integer;
var
  primeNumber: integer;
  flag: boolean;
begin
  primeNumber := primeCounter;
  flag := false;

  while not flag do
    begin

      if _set = SET1 then
        Inc(primeNumber)
      else if _set = SET2 then
        Inc(primeNumber, 2)
      else if _set = SET3 then
        Inc(primeNumber, 3)
      else if _set = SET4 then
        Inc(primeNumber, 4)
      else if _set = SET5 then
        Inc(primeNumber, 5)
      else if _set = SET6 then
        Inc(primeNumber, 6)
      else if _set = SET7 then
        Inc(primeNumber, 7)
      else if _set = SET8 then
        Inc(primeNumber, 8 + 8);

      if IsPrime(primeNumber) then
          flag := true;

    end;

    Result := primeNumber;
end;

{ Populates the prime sets with prime numbers. }
procedure TMactacHash.PopulatePrimeSets();
var
  i, primeNumber: integer;
begin
  primeNumber := 2;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET1);
      primeSet1[i] := primeNumber;
    end;

end;

And here is my output:

3 mod 1 = 0
3 mod 2 = 1
3 mod 3 = 0
4 mod 1 = 0
4 mod 2 = 0
4 mod 3 = 1
4 mod 4 = 0
...

As you can see it mods 3 and 4 over and over. Why does it do this and how can I fix it. Is there maybe a more efficient way of checking if numbers are prime or not?

Thanks in advance.

[SalmiSoft]

How have you defined primesets, SET1 to SET8 and MAX_TIMES?

In your IsPrime routine you don't need to loop all the way to Value. Looping to Value/2 will do, because any number greater than Value/2 cannot be a factor of Value.

[ddanbe]

In fact, you just have to loop to the square root of Value.

[CodeWarrior14]

type PrimeSets = (SET1, SET2, SET3, SET4, SET5, SET6, SET7, SET8);

and

primeSet1: array[0..64] of integer;
primeSet2: array[0..64] of integer;
primeSet3: array[0..64] of integer;
primeSet4: array[0..64] of integer;
primeSet5: array[0..64] of integer;
primeSet6: array[0..64] of integer;
primeSet7: array[0..64] of integer;
primeSet8: array[0..64] of integer;

[SalmiSoft]

One problem is that your isPrime method will always return FALSE because you check all factors from 1 upwards, and any value mod 1 = 0.

I am having problems understanding your code. It seems primeset2 to primeset8 are never used, and I don't follow your logic about the sets. Please post your full code -so that it will compile and run - and I'll take another look.

[CodeWarrior14]

Ok sure.

program mactac_hash;

{$APPTYPE CONSOLE}

uses
  SysUtils;

type PrimeSets = (SET1, SET2, SET3, SET4, SET5, SET6, SET7, SET8);

type TMactacHash = class
  private
    primeSet1: array[0..64] of integer;
    primeSet2: array[0..64] of integer;
    primeSet3: array[0..64] of integer;
    primeSet4: array[0..64] of integer;
    primeSet5: array[0..64] of integer;
    primeSet6: array[0..64] of integer;
    primeSet7: array[0..64] of integer;
    primeSet8: array[0..64] of integer;

    function IsPrime(value: integer): boolean;
    function GeneratePrime(primeCounter: integer; _set: PrimeSets): integer;
    procedure PopulatePrimeSets();
  public
    procedure HashFile(filename: string);
end;

{ Global variables. }
var
  mactacHash: TMactacHash;
const
  MAX_TIMES = 64;

{ Vefies if a value is prime. }
function TMactacHash.IsPrime(value: integer): boolean;
var
  i, overValue: integer;
  fValue: real;
  flag: boolean;
begin
  flag := true;
  fValue := value;

  for i := 1 to Sqrt(fValue) do
    begin

      overValue := value mod i;
      if overValue = 0 then
        flag := false;

      WriteLn(IntToStr(value) + ' mod ' + IntToStr(i) + ' = ' + IntToStr(overValue));
      readln;

    end;

  Result := flag;
end;

{ Generates the prime number. }
function TMactacHash.GeneratePrime(primeCounter: integer; _set: PrimeSets): integer;
var
  primeNumber: integer;
  flag: boolean;
begin
  primeNumber := primeCounter;
  flag := false;

  while not flag do
    begin

      if _set = SET1 then
        Inc(primeNumber)
      else if _set = SET2 then
        Inc(primeNumber, 2)
      else if _set = SET3 then
        Inc(primeNumber, 3)
      else if _set = SET4 then
        Inc(primeNumber, 4)
      else if _set = SET5 then
        Inc(primeNumber, 5)
      else if _set = SET6 then
        Inc(primeNumber, 6)
      else if _set = SET7 then
        Inc(primeNumber, 7)
      else if _set = SET8 then
        Inc(primeNumber, 8);

      if IsPrime(primeNumber) then
          flag := true;

    end;

    Result := primeNumber;
end;

{ Fill in the prime sets. }
procedure TMactacHash.PopulatePrimeSets();
var
  i, primeNumber: integer;
begin
  primeNumber := 2;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET1);
      primeSet1[i] := primeNumber;
    end;


  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET2);
      primeSet2[i] := primeNumber;
    end;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET3);
      primeSet3[i] := primeNumber;
    end;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET4);
      primeSet4[i] := primeNumber;
    end;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET5);
      primeSet5[i] := primeNumber;
    end;


  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET6);
      primeSet6[i] := primeNumber;
    end;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET7);
      primeSet7[i] := primeNumber;
    end;

  for i := 0 to MAX_TIMES do
    begin
      primeNumber := GeneratePrime(primeNumber, SET8);
      primeSet8[i] := primeNumber;
    end;
end;

procedure TMactacHash.HashFile(filename: string);
begin
  PopulatePrimeSets();
end;

{ main }
begin
  mactacHash := TMactacHash.Create;
  mactacHash.PopulatePrimeSets;
  ReadLn;
end.

Basically the program is to generate 8 sets of 64 primes. These prime are stored in the prime sets. Each set of primes has a different pattern, for set 1 you just add 1, for set 2 you add 2 ..... set 8 you add 8 until you find the next prime. This method alows me to generate bigger prime numbers and to some degree quite unique prime numbers for each set.

[SalmiSoft]

OK, so not much to correct.

Sqrt(fValue)

Which compiler are you using? I am surprised that any compiler will accept this because the loop limits should be assignment-compatible with the loop counter. Change to:

trunc(Sqrt(fValue))

On the same line, your loop checks all possible factors but that should not include 1. By definition, a prime number must be greater than 1. So if Value > 1 run your existing code but start the FOR loop from 2 instead of 1. If Vale <= 1, return FALSE. So you end up with:

  if Value > 1 then
  begin
    flag := true;
    fValue := value;
    for i := 2 to trunc(Sqrt(fValue)) do
      begin
        overValue := value mod i;
        if overValue = 0 then
          flag := false;
        WriteLn(IntToStr(value) + ' mod ' + IntToStr(i) + ' = ' + IntToStr(overValue));
        readln;
      end;
    Result := flag;
  end
  else
    Result := false;

With these changes I think your code will do what you want.

Regarding efficiency, I wouldn't worry about it. This code will be quite quick enough for any practical purposes unless you call it many many times, but why would you do that when the results will be the same each time?
If you wanted to make it more efficient without major changes, exit the loop in isPrime as soon as you get a false result. (If your compiler doesn't support the Break command, change to a while loop and check the result is still true as part of the while conditions).

[CodeWarrior14]

I'm using the old Delphi 7 compiler. All that I have at the moment.

[CodeWarrior14]

I'm still having this probelm. I modified the code a little but I still have the problem where it doesn't exit the loop when I do indeed find a prime.

program mactac_hash;

{$APPTYPE CONSOLE}

uses
  SysUtils;

const
  PRIME_SET_AMOUNT  = 8;
  PRIME_SET_SIZE = 64;

type PrimeSets = (SET1, SET2, SET3, SET4, SET5, SET6, SET7, SET8);

type TMactacHash = class
  private
    primeSets: array of array of integer;

    function GeneratePrimeNumber(_number: integer; primeSet: PrimeSets): integer;
    function ValidatePrime(number: integer): boolean;
    procedure PopulatePrimeSets();

  public
//    function HashFile(filename: string): boolean;

  published
    constructor Create(var success: boolean);
end;

{ Allocates memory for the array. }
constructor TMactacHash.Create(var success: boolean);
var
  i: integer;
begin
  success := true;

  SetLength(primeSets, PRIME_SET_AMOUNT);

  // Set the size for the 8 sets.
  for i := 0 to 7 do
    SetLength(primeSets[i], PRIME_SET_SIZE);
end;

{ Generates prime numbers. }
function TMactacHash.GeneratePrimeNumber(_number: integer; primeSet: PrimeSets): integer;
var
  isPrime: boolean;
  number: integer;
begin
  number := _number;

  // Repeat until a prime number is found.
  repeat

    if primeSet = SET1 then
      Inc(number);
    if primeSet = SET2 then
      Inc(number, 4);
    if primeSet = SET3 then
      Inc(number, 3);
    if primeSet = SET4 then
      Inc(number, 16);
    if primeSet = SET5 then
      Inc(number, 5);
    if primeSet = SET6 then
      Inc(number, 36);
    if primeSet = SET7 then
      Inc(number, 7);
    if primeSet = SET8 then
      Inc(number, 64);

    isPrime := ValidatePrime(number);

  until isPrime = true;

  Result := number;
end;

{ Checks if a number is a prime number. }
function TMactacHash.ValidatePrime(number: integer): boolean;
var
  i: integer;
begin
  if number = 2 then
    begin
      Result := true;
      Exit;
    end;

  if number > 1 then
    begin

      for i := 2 to Trunc(Sqrt(number)) do
        begin

          if number mod i = 0 then
            begin
              Result := false;
              Exit;
            end;

        end;

    end
  else
    Result := false;
end;

procedure TMactacHash.PopulatePrimeSets();
var
  i: integer;
  primeNumber: integer;
begin
  primeNumber := 2;

  // Populate the first set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET1);
      primeSets[0][i] := primeNumber;
    end;

  // Populate the second set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET2);
      primeSets[1][i] := primeNumber;
    end;

  // Populate the third set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET3);
      primeSets[2][i] := primeNumber;
    end;

  // Populate the fourth set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET4);
      primeSets[3][i] := primeNumber;
    end;

  // Populate the fith set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET5);
      primeSets[4][i] := primeNumber;
    end;

  // Populate the sixth set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET6);
      primeSets[5][i] := primeNumber;
    end;

  // Populate the seventh set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET7);
      primeSets[6][i] := primeNumber;
    end;

  // Populate the eigth set.
  for i := 0 to PRIME_SET_SIZE do
    begin
      primeNumber := GeneratePrimeNumber(primeNumber, SET8);
      primeSets[7][i] := primeNumber;
    end;
end;

var
  mactac: TMactacHash;
  b: boolean;
begin
  mactac := TMactacHash.Create(b);
  mactac.PopulatePrimeSets();
end.

It just keeps on executing the loop to find the next prime number, which it never does.

[SalmiSoft]

TMactacHash.ValidatePrime will only ever set its result to TRUE if number = 2. Change from this:

begin
  if number = 2 then
    begin
      Result := true;
      Exit;
    end;

to this:

begin
  Result := true;
  if number = 2 then
      Exit;

[CodeWarrior14]

Thanks a lot, that solved my problem.