Warum funktioniert .ToInteger hier nicht?

[Ally]

Code: Alles auswählen

var
  intX: Integer;
  strS: String;
begin
    // Das funktioniert.
    intX := StrToInt(Copy('123456789123456789', 7, 4));

    // Das funktioniert.
    strS := Copy('123456789123456789', 7, 4);
    intX := strS.ToInteger;

    // Hier gibt es eine Fehlermeldung (Error: Illegal qualifier).
    intX := Copy('123456789123456789', 7, 4).ToInteger;
    .
    .

[Socke]

Copy() ist keine normale Funktion; sie wird vom Compiler durch die jeweils aufzurufende Version z.b. für Strings oder Arrays ersetzt.
Ich würde es als Bug ansehen; du darfst es gerne im Bugtracker melden, falls es dort noch keinen passenden Eintrag gibt.

Als Workaround kannst du das Ergebnis explizit als String casten oder die TypeHelper-Methode SubString verwenden. In dieser Notation bevorzuge ich letzteres.

Code: Alles auswählen

var
  i: Integer;
begin
  i := String(copy('124', 2, 1)).ToInteger;
  i := '124'.Substring(2,1).ToInteger;    
end;

[Ally]

Hallo Socke,

vielen Dank für deine Antwort.
Im Bugtracker hatte ich nichts gefunden und deshalb hier gefragt.
Also schreibe ich mal einen Report.

Gruß Roland

[wp_xyz]

Das alt-modische

Code: Alles auswählen

    intX := StrToInt(Copy('123456789123456789', 7, 4));

funktioniert immer, ohne Type-Cast und sonstige Klimmzüge, und man hat sogar weniger zu tippen,

[Ally]

Hallo wp_xyz,

Das "alt-modische" StrToInt verwende ich auch weiterhin dort, wo der Helper nicht funktioniert.
ToInteger finde ich ganz praktisch und gut lesbar.
Und genaugenommen hat man sogar weniger zu tippen. Man setzt hinter den String-Ausdruck einen Punkt und wählt aus der Popup-Liste ToInteger.
Type-Casten und sonstige Klimmzüge sind natürlich kontraproduktiv, da gebe ich dir Recht. Dann lieber "alt-modisch".

Gruß Roland

[Warf]

Das alt-modische

Code: Alles auswählen

    intX := StrToInt(Copy('123456789123456789', 7, 4));

funktioniert immer, ohne Type-Cast und sonstige Klimmzüge, und man hat sogar weniger zu tippen,

Tatsächlich ist das etwas worüber ich mir vor kurzem mal Gedanken gemacht habe, was besser ist, Funktionen oder Methoden. Also:

Code: Alles auswählen

DoSomething(Data);
//vs
Data.DoSomething;

Und ich bin mittlerweile ziemlich klar auf der Methoden Seite. Der grund dafür ist wie wir (oder zumindest ich) Denken, ich denke sequentiell in dem was passiert, also: erst passiert a, dann passiert b, bzw als folge: Aus A folgt B. So ist das auch mit den Methoden, Data.DoSomething sagt "Nimm Data und mach Dosomething damit".
Funktionen sind allerdings umgedreht, das was man zu erst liest ist was als letztes ausgeführt wird. Also man liest bei "DoSomething(Data)" eigentlich "Mache DoSomething aus und verwende dafür Data". Grade wenn man das zusammensetzt: "DoSecond(DoFirst(Data))" sagt ja praktisch "Mach DoSecond und verwende dafür das Ergebnis von DoFirst wofür Data verwendet wird".
Und so denke ich nunmal nicht, ich denke Nimm Data, mach damit DoFisrst, und mach danach mit dem Ergebnis mach DoSecond.

Besonders aufgefallen ist mir das aufgefallen bei meinem Aktuellen Projekt, bei dem ich eine Iteratorbibliothek schreibe. Nehmen wir mal an du willst einen Byte array zu einem Hex string konvertieren, das:

Code: Alles auswählen

HexStr := Reduce<String>(Map<Byte, String>(Iterate<Byte>(arr), ByteToHex), ConcatStr);

Das nimmt jedes byte vom array, wendet ByteToHex drauf an, und fügt das ergebnis dann mit ConcatStr zusammen. Ich muss das als funktionen schreiben da der FPC mit seinen Generics zu limitiert ist (und so z.B. keine generische Map funktion als Methode eines anderen Generischen typen erlaubt).
Nehmen wir an das das unterstützt werden würde, würde das so aussehen:

Code: Alles auswählen

HexStr := arr.Iterate<Byte>.Map<Byte, String>(ByteToHex).Reduce<String>
// oder mit linebreaks um die lesbarkeit zu erhöhen
HexStr := arr.Iterate<Byte>
             .Map<Byte, String>(ByteToHex)
             .Reduce<String>(ConcatStr)

Und ich muss sagen das das 2te viel lesbarer ist als das erste. Hier kann man während man den code liest direkt verstehen was passiert. Während man beim ersten erst das komplette statement lesen muss um sich dann rückwerts durchzuarbeiten und schön im zick zack auch noch hin und her lesen muss um den Funktionsnamen und das zweite argument zu lesen.

Grundsätzlich finde ich man sollte versuchen programme so zu schreiben wie man sie lesen will, und für mich bedeutet das von links nach rechts und oben nach unten in klarer abfolge. Von daher finde ich tatsächlich mittlerweile das alles was man in Methodenschreibweise machen kann, man so auch machen sollte, vor allem wenn die chance besteht das das ganze mit anderen funktionalitäten zusammengehängt wird, den verschachtelte Funktionsaufrufe sind absolut beschissen zu lesen

[Ally]

So, die Auflösung gibt es unter https://gitlab.com/freepascal.org/fpc/s ... sues/39642

Daraus folgt das intX := UTF8Copy('12345', 1, 3).ToInteger; funktioniert.

@Warf
Ja, da geht es mir wie dir, ich finde es einfach gut lesbar.

Gruß Roland

[Warf]

Du kannst statt copy auch substring benutzen:

Code: Alles auswählen

'12345'.Substring(1, 3).ToInteger

Hat sogar den vorteil das wenn du nur ein argument angeben kannst um bis zum ende zu kopieren:

Code: Alles auswählen

'12345'.Substring(1, 4).ToInteger;
// ist das gleiche wie
'12345'.Substring(1).ToInteger;

Bei konstanten ist das natürlich ein bisschen witzlos, wobei ich da wirklich den Fomat Typhelper mag:

Code: Alles auswählen

'%d: %s'.Format([num, str]);

Aber da kann ich verstehen wenn man das nicht so schön findet.

[PascalDragon]

(und so z.B. keine generische Map funktion als Methode eines anderen Generischen typen erlaubt).

Das steht definitiv noch (und seit längerem) auf meiner ToDo-Liste...

Hat sogar den vorteil das wenn du nur ein argument angeben kannst um bis zum ende zu kopieren:

Code: Alles auswählen

'12345'.Substring(1, 4).ToInteger;
// ist das gleiche wie
'12345'.Substring(1).ToInteger;

Das geht mit Copy auch:

Code: Alles auswählen

s := Copy('12345', 1);

[Ally]

Substring funktioniert natürlich auch wieder nur mit einer String-Variablen oder einem Type-Cast.

So etwas schönes wie:

Code: Alles auswählen

intX := Edit1.Text.ToInteger;
// oder
strS := Edit1.Text.Substring(1, 2);
// oder
intX := Edit1.Text.Substring(1, 2).ToInteger;

geht halt nicht.


So geht es dann:

Code: Alles auswählen

intX := String(Edit1.Text).ToInteger;

strS := String(Edit1.Text).Substring(1, 2);

intX := String(Edit1.Text).Substring(1, 2).ToInteger;

sieht aber nicht mehr so schön aus.

Und mit "schön" meine ich lesbar, verständlich und einfach.

Eine kleine Fußangel gibt es auch noch.
Wer von Copy auf Substring umbauen möchte sollte beachten, dass Copy das erste Zeichen mit 1 indexiert und Substring mit 0.

Gruß Roland

[Winni]

Hi!

Endlose Debatten, KLassen-Quatsch und unleserlicher Code.
Und nervige Exceptions.

So mach man das:

Code: Alles auswählen

i,error : Integer;
s : string;
...
val (s,i,error);
if error <> 0 then ....

Nicht alles was neu ist, ist auch gut!

Winni

[Warf]

Ich glaube ich habe nie gesagt das Methoden da besser sind als funktionen weils neuer ist. Wenn ich mich richtig entsinne habe ich das über einen relativ ausführlichen Text diskutiert und mit Beispielen unterlegt

Aber wenn du schon val anbringst, lass mich erklären warum das so eine echt schlechte variante ist. Und nein, das hat nichts mit alter zu tun, wenn mir heute jemand code wie Val in eine Codebase committen wöllte, würde ich den merge request ablehnen egal wie neu der code ist.

Das größte problem mit Val ist der name. Val wird in den allermeisten Fällen genauso verwendet wie TryStrToInt, wie in deinem beispiel gezeigt.
Vergleicht man jetzt nur vom Namen Val und TryStrToInt, so ist ziemlich klar was die TryStrToInt macht, es versucht einen string in einen integer zu konvertieren. Es ist sogar so klar, das man, wie z.b. ich früher, die funktion ganz von selbst entdecken kann (dank autocomplete) und nur durch den Namen und signatur weiss was die funktion macht. Ich konnte die funktion finden und benutzen ohne die dokumentation zu brauchen. Das ist gutes Design und eine eigenschaft die man so viel wie möglich haben will.
Val auf der anderen seite hat einen komplett generischen namen. Es wurde sogar darauf verzichtet den begriff value auszuschreiben. So viel Zeit sollte wohl sein die paar extra Buchstaben zu tippen, vor allem mit modernen IDEs die über intelligente autovervollständigung einem das tippen von Namen sowieso zum großteil abnehmen können. Wir können es uns leisten funktionen bedeutungsvolle namen zu geben. Keine deadline wird verletzt weil die programmierer mehr als 3 buchstaben für funktionsnamen tippen müssen.

Während ich TryIntToStr selbst entdeckt und ohne jedwede dokumentation verstanden habe, erinnere ich mich das als ich das erste mal Val in code gesehen habe ich erst mal nachschauen musste was die funktion eigentlich macht.
Allein das disqualifiziert val schon einmal gegenüber TryStrToInt. Für code wie dein Beispiel gibt es keinen grund val über TryStrToInt zu bevorzugen.

Dann, abgesehen von dem herzlichst bescheidenen namen, ist Val aus einem mir unerfindlichen grund eine prozedur die ausgabe als parameter gibt. Es gibt bereits ein konzept für codeblöcke die ergebnisse zurückgeben sollen, das nennt sich Funktion mit return value.
Zum Vergleich nehmen wir mal wieder TryStrToInt, was eine funktion ist:

Code: Alles auswählen

if TryStrToInt(s, i) then

Dein code braucht 2 zeilen und eine zusätzliche variable, du brauchst also mehr code (mehr im sinne von Strukturen nicht unbedingt zeichen, auch wenn du da auch mehr hast) um die gleiche Information darzustellen. Wenn es um code Qualität und komplexität geht borgt man sich gerne den begriff der signal to noise Ratio (SNR) aus der Informationstheorie, also wie viel Information (signal) is dort im verhältnis zu sachen drum herum (die noise) wie z.b. boilerplate code. Je mehr code du also brauchst um die selbe menege an information zu vermitteln desto schlechter die SNR und damit desto schwerer ist der code zu verstehen. Und was das angeht ist Val deutlich komplexer als TryStrToInt bei der gleichen Information die der code darstellt (also: converte string on int und check obs funktioniert hat)

So herzlichen glückwunsch, du hast es wirklich geschafft das wohl schlechtest mögliche Beispiel zu finden. Es gibt viele gute gründe einen expliziten error check in die conversion einzubauen, es gibt keinen einzigen guten grund aber Val statt TryStrToInt zu verwenden

PS: ich gehe davon aus das die variablen Namen s und i nur als Beispiel sind, in produktiv code sollten variablen (ausser eventuell loop variablen) natürlich auch vernünftige namen bekommem, hier gilt das gleiche was ich oben zu Val geschrieben habe

[BeniBela]

Aber wenn man in den Code guckt, macht TryStrToInt nur eine Kopie vom String und ruft damit Val auf. Das ist unnötig langsam.

Und in dem Originalbeispiel ist auch schon ein Copy('123456789123456789', 7, 4). Da macht man zwei Kopien, die man nicht braucht, wenn man nur eine Zahl haben will.




In den letzten Tagen habe ich an einem Stringview gearbeitet

Das löst alle Probleme und ist lesbarer als Val. Das ist die Stringverarbeitung der Zukunft :

Code: Alles auswählen

var
  intX: Integer;
  strS: String;
  v: TPCharView;
begin
    v := '123456789123456789'.pcharView;
    v.rightOfFirst(6);
    v.leftOf(v.data + 4);
    v.toIntDecimalTry(intX);
    writeln(intX);

Copy(.., 7, 4) wird zu rightOfFirst(6)/leftOf(v.data + 4);, d.h. .erst entfernt man die ersten 6 Zeichen und setzt dann die Länge auf 4. Das sollte lesbarer als Copy/Substring sein, denn man muss nicht überlegen, ob der Index bei 0 oder 1 beginnt. Nur für leftOf(v.data + 4) braucht es wohl noch eine weitere Methode, das müsste dann leftWithFirst(4) heißen (oder eventuell setLength(4), aber dann weiß man nicht, bekommt man die ersten 4 oder die letzten 4 Zeichen, wäre also schlechter lesbar als leftWithFirst )

[Warf]

Lustig, ich hab auch mal an nem View gearbeitet: https://github.com/Warfley/Recutils/tree/master/range
Bzw bin sogar wieder dabei daran weiter zu arbeiten in nem neuen repo. Auch wenn der fokus tatsächlich mehr utulity ist als unbedingt performance, so ist unnötiges kopieren des speichers doch was was nicht unbedingt nötig ist

Aber so sehr ich aich performance spielereien mag, muss ich doch sagen das ich erst einmal an einem Pascal Programm gearbeitet hab wo performance kritisch war.
Und meine Philosophie ist das man so wenig arbeit in extra performance reinstecken sollte wie möglicj bis zu dem punkt wo man wirklich auf solche probleme trifft
Und dann sollte man mit nem profiler seinen code analysieren und gezielt optimieren

Ansonsten hat einfachheit und lesbarkeit des codes M.E. immer Vorrang. Wäre natürlich schön wenn der RTL code schon von anfang an optimal wäre, das ist er leider nicht (eine der sachen die ich an der C++ STL so mag, wenn man was benutzt kann man sich sicher sein das das hoch optimiert ist)

[Winni]

...
So herzlichen glückwunsch, du hast es wirklich geschafft das wohl schlechtest mögliche Beispiel zu finden. Es gibt viele gute gründe einen expliziten error check in die conversion einzubauen, es gibt keinen einzigen guten grund aber Val statt TryStrToInt zu verwenden

Die Funktionen StrToInt, StrToFloatr, TryStrToInt etc benutzen intern alle Val. Dazu lösen sie evtl noch eine Exception aus, die aufwendig mit einem (oder zwei) Try abgefangen werden muss. Aufgeblasener, geschwätzigem Code erzwungenermaßen. Wenn Du wirklich eine Funktion statt einer Procedure für Val benötigst, schnitzt Du Dir das mit 4 Zeilen:

Code: Alles auswählen

function  Str2Int (s: string; var error: Integer) : integer;
begin
val(s,result,error);
end 

PS: ich gehe davon aus das die variablen Namen s und i nur als Beispiel sind, in produktiv code sollten variablen (ausser eventuell loop variablen) natürlich auch vernünftige namen bekommem, hier gilt das gleiche was ich oben zu Val geschrieben habe

Naseweis