3
\$\begingroup\$

The following pascal function (compiled with Delphi) will split strings. It works perfectly, but how to improve the code? For example, to avoid using the repeat-until loop.

type
  TSarray = array of string;

function Split(Texto, Delimitador: string): TSarray;

var
  o: integer;
  PosDel: integer;
  Aux: string;

begin

  o := 0;
  Aux := Texto;
  SetLength(Result, Length(Aux));

  repeat

    PosDel := Pos(Delimitador, Aux) - 1;

    if PosDel = -1 then
    begin
      Result[o] := Aux;
      break;
    end;

    Result[o] := copy(Aux, 1, PosDel);
    delete(Aux, 1, PosDel + Length(Delimitador));
    inc(o);
  until Aux = '';
end;

Example:

var texto,deli:string;
    all_array:TSarray;
begin
deli := 'test';
texto := deli+'hi world 1'+deli+'hi world 2'+deli;
end;

all_array := Split(texto,deli);
ShowMessage(all_array[1]);
ShowMessage(all_array[2]);
end;

My plan is to use no classes, only the "uses" default

What alternatives do I have to repeat-until?

\$\endgroup\$
  • \$\begingroup\$ Can you please add some example inputs / outputs of what your code is doing? \$\endgroup\$ – Simon Forsberg Feb 24 '15 at 15:01
  • \$\begingroup\$ Ok , post updated. \$\endgroup\$ – Matt Olsen Feb 24 '15 at 15:10
  • \$\begingroup\$ I meant add some example inputs / outputs in pure text form, for those that do not have a Delphi compiler around. \$\endgroup\$ – Simon Forsberg Feb 24 '15 at 16:40
  • \$\begingroup\$ Did you consider using recursion? \$\endgroup\$ – Mawg Apr 13 '15 at 12:15
  • \$\begingroup\$ Recursion with undefined depth, that is unknown count of delimiter occurrences here, is a bad idea. \$\endgroup\$ – René Hoffmann Jun 25 '15 at 13:43
3
\$\begingroup\$

First, I don't quite understand what you mean with not using repeat-until, because in order to split string you have to use some kind of loop.

I assume that you want to have some speed improvements of your code.

If you are not changing string parameters passed to routine you should declare them as const parameters to make call run faster

function Split(const Texto, Delimitador: string): TSarray;

You are reserving way too much memory with SetLength(Result, Length(Aux)); You can freely increase length of resulting array as you go SetLength(Result, Length(Result) + 1); and it will also run faster.

And finally, copying string you are splitting and then deleting parts of the string is not optimal implementation. You should be using overloaded version of Pos function where third argument is offset from which it starts searching, and eliminate Aux string completely.

Also, using o as variable name is not recommended because it can easily be misread as 0, so I replaced it with i, that is commonly used as index variable.

So final code with all optimizations implemented might look like this:

function Split(const Texto, Delimitador: string): TSarray;
var
  i: integer;
  Len: integer;
  PosStart: integer;
  PosDel: integer;
begin
  i := 0;
  SetLength(Result, 1);
  Len := Length(Delimitador);
  PosStart := 1;
  PosDel := Pos(Delimitador, Texto);
  while PosDel > 0 do
    begin
      Result[i] := Copy(Texto, PosStart, PosDel - PosStart);
      PosStart := PosDel + Len;
      PosDel := Pos(Delimitador, Texto, PosStart);
      inc(i);
      SetLength(Result, i + 1);
    end;
  Result[i] := Copy(Texto, PosStart, Length(Texto));
end;
\$\endgroup\$

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.