GetLocaleInfo() Aufruf, ist es so richtig?

[KodeZwerg]

Hallo, ich würde mich gerne absichern ob folgender Code korrekt ist:

Delphi-Quellcode:

function ThousandSeparator : String;
var
  buf: PChar; // temp puffer
begin
  Result := '#'; // falsch initialisieren, gegebenfalls später mit einem Default-Wert bei Fehler ersetzen
  try
  buf := StrAlloc(10); // puffer eine größe zusichern
  GetLocaleInfo(
    LOCALE_SYSTEM_DEFAULT,
    LOCALE_STHOUSAND,
    buf,
    10); // GetLocaleInfo() direkt abfragen ohne die Länge vom Trennzeichen zu kennen, aber über 10 sollten es wohl nicht werden
  finally
   Result := StrPas(buf); // in einen String konvertieren
   StrDispose(buf); // speicher bereinigen
  end;
end;

Ziel soll sein das ich in einem String das/die Zeichen für's Lokale Tausender-Trennzeichen bekomme.
Oder ist es generell so falsch und es gibt aktuellere Methoden um das Zeichen (als String) zu bekommen?

[Zacherl]

Zitat:

Zitat von KodeZwerg (Beitrag 1397884)

Oder ist es generell so falsch und es gibt aktuellere Methoden um das Zeichen (als String) zu bekommen?

Delphi hat diese Schnittstelle gekapstelt:
http://docwiki.embarcadero.com/CodeE...ttings_(Delphi)

Ansonsten noch zum WinAPI Ansatz:
"Theoretisch" kann

Delphi-Quellcode:

StrAlloc

fehlschlagen. In diesem Falle läuft dein

Delphi-Quellcode:

StrDispose

im

Delphi-Quellcode:

finally

Block ins Leere. Und die fixe Länge von 10 wird sicherlich auch funktionieren, aber korrekter wäre es die API einmal mit

Delphi-Quellcode:

nil

Buffer und 0 Länge aufzurufen und den Rückgabewert zu verwenden. Alternativ einen fixen Anfangsbuffer, Rückgabe prüfen, GetLastError auf

Delphi-Quellcode:

ERROR_INSUFFICENT_BUFFER

prüfen, Buffer verdoppeln, Aufruf wiederholen, etc.

[KodeZwerg]

Wäre es so korrekt? (mit Verzicht auf WinApi)

Delphi-Quellcode:

function ThousandSeparator : String;
var
  FS: TFormatSettings;
begin
  FS := TFormatSettings.Create;
  Result := FS.ThousandSeparator;
end;

Muss ich da noch Speicher bereinigen?

[Zacherl]

Zitat:

Zitat von KodeZwerg (Beitrag 1397889)

Muss ich da noch Speicher bereinigen?

Nee, ist ja ein

Delphi-Quellcode:

record

. Alternativ beinhaltet

Delphi-Quellcode:

System.SysUtils

auch die globale Variable

Delphi-Quellcode:

FormatSettings

, die du direkt verwenden kannst.

[günni0]

Zitat:

Zitat von KodeZwerg (Beitrag 1397889)

Wäre es so korrekt? (mit Verzicht auf WinApi)

Delphi-Quellcode:

function ThousandSeparator : String;
var
  FS: TFormatSettings;
begin
  FS := TFormatSettings.Create;
  Result := FS.ThousandSeparator;
end;

Muss ich da noch Speicher bereinigen?

So ungefähr mache ich es auch. Bisher hat sich, trotz internationalem Gebrauch meines Programms, keiner beschwert. Ich erstelle TFormatSettings aber nur einmalig bei Programmstart und nutze die so gewonnenen Daten dann weiter.

[KodeZwerg]

Nun aber hoffentlich Ok:

Delphi-Quellcode:

function ThousandSeparator : String;
begin
  Result := FormatSettings.ThousandSeparator;
end;

Vielen Dank für so schnelle Hilfe!

[günni0]

Jetzt noch alles in eine eigene Unit und alles ist sauber, schön und leicht erweiterbar (jedenfalls würde ich es so machen).

Delphi-Quellcode:

unit MyUnitX;

interface

type
 TUnitX = record
 private
 var
  MyFormatSettings: TFormatSettings;
 public
  class function ThousandSeparator : string; static;
 end;

implementation

var
 UnitX: TUnitX;

class function TUnitX.ThousandSeparator : String;
begin
  Result := FormatSettings.ThousandSeparator;
end;

initialization

UnitX.MyFormatSettings := TFormatSettings.Create(GetUserDefaultLCID);

So eine ähnliche Unit habe ich. Grund für MyFormatSettings ist, weil ich FormatSettings überschreibe und immer noch die originalen Werte haben möchte.

[KodeZwerg]

Ich portiere gerade Code der auf System.ThousandSeperator aufbaute, kommt nur in einer Unit vor, hab da nun meine funktion drinn, klappte auch beim ersten Post, ich wollte mich nur absichern das ich da kein Fehler mache, da ich die WinApi Antwort leider nicht ganz Verstanden habe erschien mir das was ich Verstanden habe einfacher umzusetzen. :mrgreen:
[EDIT]Also das mit dem StrDispose() das es fehlschlagen könnte hab ich Verstanden aber den Rest noch nicht.[/EDIT]

[himitsu]

Zitat:

Zitat von KodeZwerg (Beitrag 1397884)

Hallo, ich würde mich gerne absichern ob folgender Code korrekt ist
...
Oder ist es generell so falsch und es gibt aktuellere Methoden um das Zeichen (als String) zu bekommen?

Gegenfrage: Warum hörst du nicht auf deinen Compiler? (vonwegen Variable nicht initialisiert)

Delphi-Quellcode:

buf := StrAlloc(10); // puffer eine größe zusichern

gehört vor das TRY
und

Delphi-Quellcode:

Result := StrPas(buf);

gehört vor Finally oder hinter das End.

Falls StrAlloc knallt, führst du dennoch StrDispose aus, es knallt nochmal und verfälscht den eigentlichen Fehler
und wenn es knallt, greifst du auch noch auf buf zu, was ebenfalls nicht gut ist.

UND, diese API löst keine Exceptions aus,
also hilft Try-Finally/Except garnichts.
Und falls es doch knallt, dann hattest DU "buf" falsch/nicht initialisiert
oder es ist so viel kaputt, dass eine Fehlerbehandlung eh sinnlos ist. (ist = war vorher schon)

Zitat:

falsch initialisieren, gegebenfalls später mit einem Default-Wert bei Fehler ersetzen

Und wie willst du erkennen, ob es einen Fehler gab, wenn du das Result dieser API nicht auswertest?


PS: Wenn ich Code als "Delphi/Pascal" implementiere, dann lasse ich das C-typische StrAlloc weg und nutzte einen passenden Delphi-String (AnsiString/UnicodeString/String)
und das StrDispose macht Delphi für mich.
Nach der API mit SetLength oder einem billigen PChar-Cast den String kürzen, falls der Inhalt doch kleiner war.

Oder ich nutze ein statisches Char-Array und SetString

[KodeZwerg]

So war mein Ansatz nach Zacherl's erstem Post:

Delphi-Quellcode:

function ThousandSeparator : String;
var
  buf: PChar;
begin
  Result := ','; // falls was schiefläuft einen default Wert
  buf := StrAlloc(10);
  if GetLastError() = ERROR_SUCCESS then
   if GetLocaleInfo(
       LOCALE_SYSTEM_DEFAULT,
       LOCALE_STHOUSAND,
       buf,
       10) <> 0 then Result := StrPas(buf);
   if GetLastError() <> ERROR_INSUFFICIENT_BUFFER then StrDispose(buf);
end;

[Zacherl]

Nee das ist auch Murks :-D In etwa so müsste es aussehen:

Delphi-Quellcode:

var
  Buffer: PChar;
  BufferLen: Integer;
begin
  BufferLen := GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, nil, 0);
  if (BufferLen = 0) then
  begin
    RaiseLastOSError;
  end;
  Buffer := StrAlloc(BufferLen);
  if (not Assigned(Buffer)) then
  begin
    raise EOutOfMemory.Create('StrAlloc failed');
  end;
  try
    if (GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, Buffer, BufferLen) <> BufferLen) then
    begin
      RaiseLastOSError;
    end;
    Result := StrPas(Buffer);
  finally
    StrDispose(Buffer);
  end;
end;

[günni0]

Was soll bei

Delphi-Quellcode:

 Result := FormatSettings.ThousandSeparator;

denn schiefgehen?

[Zacherl]

Zitat:

Zitat von günni0 (Beitrag 1397903)

Was soll bei

Delphi-Quellcode:

 Result := FormatSettings.ThousandSeparator;

denn schiefgehen?

Nichts. Wenn man die gekapselte Version nimmt, ist man so ziemlich auf der sicheren Seite. Das "Schiefgehen" bezog sich auf den WinAPI Ansatz bzw. insbesondere das

Delphi-Quellcode:

StrDispose

im Falle eines vorher erfolglosen

Delphi-Quellcode:

StrAlloc

s.

[himitsu]

Zitat:

Delphi-Quellcode:

  buf := StrAlloc(10);
  if GetLastError() = ERROR_SUCCESS then

Was soll GetLastError liefern, wenn StrAlloc keinen Fehler lieferte?

Etwas Zufälliges und zwar "laut Definition" den Fehlercode irgendeiner WinAPI, welches davor als Letztes einen Fehler lieferte.


Genau darum steht auch niemals "Ergebnis siehe GetLastError,
sondern "prüfe Result und wenn was schief lief, dann siehe GetLastError".

Zitat:

Delphi-Quellcode:

if GetLocaleInfo(
       LOCALE_SYSTEM_DEFAULT,
       LOCALE_STHOUSAND,
       buf,
       10) <> 0 then Result := StrPas(buf);
   if GetLastError() <> ERROR_INSUFFICIENT_BUFFER then

Wenn GetLocaleInfo erfolgreich war, dann war in StrPas die letzte WinAPI, aber vielleicht kommt das Result auch von irgendeiner WinAPI von irgendwann weit davor.

[KodeZwerg]

Danke nochmals für die korrekte Handhabung per Api @Zacherl, ich hatte den Ansatz eigentlich nur gepostet wegen voriger Aussage warum ich nicht auf Compiler-Meldungen achte, das hatte ich da auch wenn Code falsch ist :-)

[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

Delphi-Quellcode:

finally

Abschnitt in jedem Falle ausgeführt wird (selbst bei

Delphi-Quellcode:

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

Delphi-Quellcode:

if Assigned(Buffer) then

sowieso immer True geliefert hätte.
Und zusammen mit dem Finally wäre StrDispose dann doppelt ausgeführt worden.

Zitat:

Zitat von Zacherl (Beitrag 1397915)

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? :thumb: