GetLocaleInfo() Aufruf, ist es so richtig?

[KodeZwerg]

Könnte man Dein Beispiel auch so abändern oder ginge dabei wieder etwas schief?

Delphi-Quellcode:

function ThousandSeparator : String;
const
 Default = ','; // Bei Fehler ein Default Wert nutzen um Meldungen zu überspringen
var
  Buffer: PChar;
  BufferLen: Integer;
begin
  BufferLen := GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, nil, 0);
  if (BufferLen = 0) then
  begin
    Result := Default;
    System.SetLastError(ERROR_SUCCESS);
    Exit;
    // RaiseLastOSError;
  end;
  Buffer := StrAlloc(BufferLen);
  if (not Assigned(Buffer)) then
  begin
    Result := Default;
    System.SetLastError(ERROR_SUCCESS);
    Exit;
    // raise EOutOfMemory.Create('StrAlloc failed');
  end;
  try
    if (GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, Buffer, BufferLen) <> BufferLen) then
    begin
      Result := Default;
      if Assigned(Buffer) then StrDispose(Buffer);
      System.SetLastError(ERROR_SUCCESS);
      Exit;
      // RaiseLastOSError;
    end;
    Result := StrPas(Buffer);
  finally
    StrDispose(Buffer);
  end;
end;

[Zacherl]

Würde soweit funktionieren. Die extra Abfrage bezüglich String freigeben kannst du dir aber sparen, da der finally Abschnitt in jedem Falle ausgeführt wird (selbst bei Exit ). Und SetLastError lass am besten auch komplett weg.

[KodeZwerg]

Vielen Dank! Ich habe nun das Extra StrDispose() und die SetLastError() Aufrufe entfernt.

[himitsu]

Abgesehn davon, dass if Assigned(Buffer) then sowieso immer True geliefert hätte.
Und zusammen mit dem Finally wäre StrDispose dann doppelt ausgeführt worden.

Zitat von Zacherl:

Und SetLastError lass am besten auch komplett weg.

Es heißt ja auch SetLastError und nicht LastState.
IMHO wäre SUCCESS laut Definition somit falsch.

[KodeZwerg]

Mit dem "System.SetLastError(ERROR_SUCCESS);" wollte ich halt Verhindern das aus dem Programm heraus irgendwelche Fehler-Meldungen aufploppen könnten wenn die Funktion ThousandSeparator aufgerufen wird.
Die Delphi Hilfe über RaiseLastOSError brachte mich darauf.
Die Funktion soll nur ein Ergebnis abliefern aber nie einen Fehler, total transparent, so als ob es Sie nicht gibt.

[Jasocul]

Da mich die Exit-Anweisungen in dem Code gestört haben, musste ich einfach den Source umschreiben. Jedes der Exits hätten einfach in ein else gepasst.
Dabei ist mir dann aufgefallen, dass der Source insgesamt noch einfacher geschrieben werden kann, wenn man den Default-Wert als erstes setzt und nur noch die die Sachen stehen lässt, die diesen verändern würden. Das SetLasteError habe ich auch weg gelassen, da ich die Notwendigkeit an der Stelle nicht erkennen kann. Dabei ist dann dieses bisschen Source übrig geblieben:

Delphi-Quellcode:

function ThousandSeparator : String;
const
  Default = ','; // Bei Fehler ein Default Wert nutzen um Meldungen zu überspringen
var
  Buffer: PChar;
  BufferLen: Integer;
begin
  Result := Default;
  BufferLen := GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, nil, 0);
  if (BufferLen > 0) then
  begin
    Buffer := StrAlloc(BufferLen);
    if Assigned(Buffer) then
    begin
      try
        if (GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, Buffer, BufferLen) = BufferLen) then
        begin
          Result := StrPas(Buffer);
        end;
      finally
        StrDispose(Buffer);
      end;
    end;
  end;
end;

Der Source kann nach kürzer werden, wenn man die nicht notwendigen "begin" und "end" bei zwei der if-Abfragen weglässt.

Zu SetLastError und RaiseLastOSError:
Da hier ein Default-Wert gesetzt wird, sind diese beiden Anweisungen nicht erforderlich. Gäbe es keinen Default-Wert, könnte das Ergebnis undefiniert sein. Dann wären die Anweisungen möglicherweise sinnvoll.

[KodeZwerg]

Danke für's Upgrade!

[himitsu]

Zitat:

RaiseLastOSError brachte mich darauf

Cool, wieder ein Programm, dass einem die sinnvollste Fehlermeldung anzeigt?