String mit alphabetisch geordneten Substring

SeppJ

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

und kommentare fehlen auch noch!

Ernsthafte Frage: Würdest du wirklich sagen, mein Code oben braucht Kommentare? Wenn ja, welche?

wob

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

und kommentare fehlen auch noch!

Sind Variablen gut benannt und die Funktionen kurz, erübrigen sich häufig Code-Kommentare. Daher lese ich lieber Code ohne Kommentare mit gut benannten Variablen als kommentierten Code mit schlecht benannten Variablen. Außerdem passiert es schnell, dass Kommentare nicht mehr stimmen. Beim Ändern von Code muss man auch höllisch aufpassen, weil Kommentare auch deutlich vor oder hinter einer Änderung vorkommen können.

Einzig die Funktion selbst sollte gut dokumentiert sein, denn aus getLongest kann man nicht ablesen, was es tut. Wie wäre es mit longestOrderedSubstring(string, Compare=std::less) oder ähnlich - meinetwegen hier auch erstmal longestAscendingSubstring(string)? Auf jeden Fall verdient jede Funktion, die keine kleine Helper-Funktion ist, eine Beschreibung ihrer Funktion und ihrer Ein- und Ausgabewerte.

LeMace

@SeppJ sagte in String mit alphabetisch geordneten Substring:

Ernsthafte Frage: Würdest du wirklich sagen, mein Code oben braucht Kommentare? Wenn ja, welche?

Das first_ordered_subsequence_length() nicht mit dem Leerstring aufgerufen werden darf würde ich hier kommentieren...

wob

@LeMace Warum ist das Ergebnis nicht einfach 0? Bzw. der Leerstring? (je nachdem, ob du die Länge oder den String selbst haben willst) Es wäre überraschend, wenn diese Funktion die Anforderung "kein Leerstring" hätte.

LeMace

@wob
Ich rede von folgender Funktion:

size_t first_ordered_subsequence_length(const char *sequence)
{
  char previous_char = *++sequence;
  size_t length = 1;
  while(*sequence)
    {
      char current_char = *sequence;
      if (current_char >= previous_char)
        length += 1;
      else
        return length;
      previous_char = current_char;
      ++sequence;
    }
  return length;
}

Wenn die mit "" aufgerufen wird rödelt die while-Schleife doch im Nirvana rum. Oder übersehe ich da etwas?

wob

Und das könnte man nicht durch ein einfaches if (!*sequence) return 0; beheben? Oder indem man previous_char und length beide auf 0 setzt und dann gleich in die while-Schleife geht?

SeppJ

Es ist halt eine interne Funktion, die nicht für den Aufruf durch Anwender gedacht ist. Innerhalb eines einzelnen Codeblocks ist das etwas schwer kenntlich zu machen, aber im Header würde nur die andere Funktion stehen. Innerhalb meiner "Library" kann ich natürlich sicherstellen, dass meine eigenen Annahmen nicht verletzt sind.

LeMace

@SeppJ Deshalb würde ich das "nur" über einen Kommentar lösen. Damit die Kollegen das sofort sehen können.

@SeppJ sagte in String mit alphabetisch geordneten Substring:

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

und kommentare fehlen auch noch!

Ernsthafte Frage: Würdest du wirklich sagen, mein Code oben braucht Kommentare? Wenn ja, welche?

schwierig, vielleicht würde ich in zeile 8 neben dem schleifenkopf irgendwie schreiben, dass in der schleife das momentane zeichen mit dem nächsten zeichen verglichen wird, um die anzahl der geordneten zeichen zu ermitteln, oder in zeile 27, dass die gesamte zeichenkette nach geordneten zeichenketten durchsucht und die position der längsten herausgesucht wird.
also ich musste da schon so einige zeit überlegen, was da eigentlich gemacht werden soll. dass dinge wie a = 8; //weise a den wert 8 zu totaler quatsch sind, ist mir schon klar, aber bei blöcken hilft das schon beim verständnis.

EinNutzer0

Ihr habt völlig recht gehabt mit dem Leerstring... und so wäre es perfekt:

#include <iostream>
#include <string>

std::string getLongest(const std::string &str)
{
    int32_t a[4] = {0};
    for (size_t i = 1; i < str.length(); i++)
    {
        if (str[i - 1] <= str[i])
        {
            a[0]++;
            if (a[0] > a[1])
            {
                a[1] = a[0];
                a[2] = a[3];
            }
        }
        else
        {
            a[0] = 1;
            a[3] = i;
        }
    }
    return str.substr(a[2], a[1]);
}

int main()
{
    std::cout << getLongest("") << std::endl;
    std::cout << getLongest("kotafgovlav") << std::endl;
    std::cout << getLongest("dcba") << std::endl;
}

Ich hab mich bewusste gegen lange, sprechende, lokale Variablennamen entschieden, weil diese hierbei unnütz wären.

@EinNutzer0 sagte in String mit alphabetisch geordneten Substring:

Ich hab mich bewusste gegen lange, sprechende, lokale Variablennamen entschieden, weil diese hierbei unnütz wären.

der grund, warum du solche aufgaben programmierst, besteht darin, das programmieren zu erlernen. da geht es weniger darum, irgendeine lösung für ein problem zu entwickeln, und mehr darum, diese sauber, übersichtlich, verständlich und nachvollziehbar zu erstellen, d.h. vernünftig einrücken, vernünftig kommentieren, vernünftige funktions- und variablennamen, vernünftiger programmablauf usw.. es ist nur mit viel rätselraten zu erkennen, was du in zeile 14 und zeile 20 eigentlich machst und wenn das array dann auch noch a heißt, wird das ganze noch komplizierter.

wenn du das verstehst, ist es schön, im berufsleben wirst du irgendwann ein besseres angebot mit doppeltem gehalt bekommen, und irgendjemand anderes muss dann damit weiterarbeiten, oder du wirst vor so ein projekt gesetzt und ärgerst dich damit ab.

@wob sagte in String mit alphabetisch geordneten Substring:

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

und kommentare fehlen auch noch!

Sind Variablen gut benannt und die Funktionen kurz, erübrigen sich häufig Code-Kommentare. Daher lese ich lieber Code ohne Kommentare mit gut benannten Variablen als kommentierten Code mit schlecht benannten Variablen.

wenn du aber für jede teilaufgabe eine extra funktion erstellst, stehst du irgendwann vor einem unübersichtlichen berg aus funktionen. deshalb ist es manchmal sinnvoll, mehrere aufgaben in einer funktion unterzubringen und jeder aufgabe eine überschrift zu verpassen.

Außerdem passiert es schnell, dass Kommentare nicht mehr stimmen. Beim Ändern von Code muss man auch höllisch aufpassen, weil Kommentare auch deutlich vor oder hinter einer Änderung vorkommen können.

also ähm dokumentation macht 2/3 der arbeit aus, oder wie war das?

EinNutzer0

ok, +1 dafür. aber wenn man dabei wirklich jedes if und jedes else kommentieren würde, würde das das alles sehr aufblähen.

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

also ähm dokumentation macht 2/3 der arbeit aus, oder wie war das?

Kommentare sind eine kleine Teilmenge der Dokumentation.

EinNutzer0

Ach Scheiße, wenn man getLongest aus #31 mit dem string "ab" aufruft, wird nur a zurückgegeben... Also wenn u. a. die Zeichenkette nur aufeinanderfolgende Zeichen enthält, wird das letzte Zeichen abgeschnitten.

Das ist blöd, deswegen taugt das ja doch nix.

Deswegen ist es aus meiner Sicht sinnvoll, viele Testfälle dafür zu formulieren.

@EinNutzer0 deshalb musst du immer abwägen, ob das sinnvoll ist. aber

if(...) //kommentar, der beschreibt, was bei erfüllung der bedingung passieren soll, und auf eine seite passt
{
}
else //kommentar der beschreibt, was bei nicht-erfüllung der bedingung passieren soll, und auf eine seite passt
{
}

fügt nicht eine einzige zeile hinzu und

int MeineFunktion()
{
     /* (optional) diese funktion erledigt diese und jene aufgabe 
        durch erfüllung der teilaufgaben a und b */

     //teilaufgabe a: berechnung von teilergebnis sowieso 1
     int a, b, c;
     a = 2 * b + 3 * c;

     //teilaufgabe b: berechnung von teilergebnis sowieso 2
     int d, e, f;
     d = 6 * e + 3 * f;

     //zusammenfügen der zwischenergebnisse
     int g = a * d;
}

wird weder durch die einleitung ganz oben, noch durch die jeweils eine zeile mehr auch nicht unübersichtlicher, aber wenn der kommentar vernünftig gewählt wird, weißt du sofort, was da gemacht wird.

EinNutzer0

Nochmal eine "Korrektur" zu #31:

#include <iostream>
#include <string>

std::string getLongest(const std::string &str)
{
    int32_t a[4] = {0};
    for (size_t i = 1; i < str.length(); i++)
    {
        if (str[i - 1] <= str[i])
        {
            a[0] = a[0] ? a[0] + 1 : 2;
            if (a[0] > a[1])
            {
                a[1] = a[0];
                a[2] = a[3];
            }
        }
        else
        {
            a[0] = 0;
            a[3] = i;
        }
    }
    return str.substr(a[2], a[1]);
}

int main()
{
    std::cout << getLongest("") << std::endl;
    std::cout << getLongest("ab") << std::endl;
    std::cout << getLongest("ba") << std::endl;
    std::cout << getLongest("abba") << std::endl;
    std::cout << getLongest("bbaa") << std::endl;
    std::cout << getLongest("dcbaa") << std::endl;
    std::cout << getLongest("kotafgovlav") << std::endl;
    std::cout << getLongest("dcba") << std::endl;
}

Schlangenmensch

@Wade1234 sagte in String mit alphabetisch geordneten Substring:

wenn du aber für jede teilaufgabe eine extra funktion erstellst, stehst du irgendwann vor einem unübersichtlichen berg aus funktionen. deshalb ist es manchmal sinnvoll, mehrere aufgaben in einer funktion unterzubringen und jeder aufgabe eine überschrift zu verpassen.

Was? Das ist mir ja noch nie untergekommen. Egal in welchem Zusammenhang, es ist immer besser Funktionalitäten in einzelne funktionale Einheiten zu trennen. Das erhöht nicht nur die Übersicht, sondern auch die Wiederverwendbarkeit und Testbarkeit.

wob

@EinNutzer0 sagte in String mit alphabetisch geordneten Substring:

Ich hab mich bewusste gegen lange, sprechende, lokale Variablennamen entschieden, weil diese hierbei unnütz wären.

Willst du uns vergesäßen?

fairiestoy

@SeppJ sagte in String mit alphabetisch geordneten Substring:

Mal angenommen, du könntest mich nicht fragen (Ja, schaut richtig aus): Wie würdest du prüfen, ob es wirklich korrekt ist?

Etwas spät ^^ Habe das dennoch jetzt dazwischen geworfen.

Prinzipiell sage ich jetzt einfach, es mit zufälligen Daten füttern, ausgeben lassen und diese vergleichen, ob die Ausgabe mit dem übereinstimmt, was ich erwarte.

Beispiel in Python (nicht als gutes Python Beispiel zu verwenden):

import subprocess as sp
import random, string

EX_NAME = "test.exe"
DATA_SIZE = 70
RESULTS = 32

def create_randstring( ssize ):
    tmp = ''
    for i in range( 0, ssize ):
        tmp += chr( random.randint( 1, 255 ) )
    return tmp

def create_randASCIIstring( ssize ):
    tmp = ''
    for i in range( 0, ssize ):
        tmp += random.choice( string.ascii_letters + string.digits )
    return tmp

def main( ):
    global EX_NAME
    global DATA_SIZE
    global RESULTS
    with open( "log.txt", "wb" ) as dout:
        for i in range( 0, RESULTS ):
            tmp = create_randASCIIstring( DATA_SIZE )
            tmp_r = sp.run( [EX_NAME, tmp], stdout=sp.PIPE )
            dout.write( b"Raw Data: " + bytes( tmp, 'utf-8' ) + b"\r\nResult: " + tmp_r.stdout[:-2] + b"\r\n" )
            tmp = create_randstring( DATA_SIZE )
            tmp_r = sp.run( [EX_NAME, tmp], stdout=sp.PIPE )
            dout.write( b"Raw Data: " + bytes( tmp, 'utf-8' ) + b"\r\nResult: " + tmp_r.stdout[:-2] + b"\r\n" )

if __name__ == '__main__':
    main()

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>

size_t step_subsequence( const char *sequence )
{
  if( !sequence || !*sequence ){ return 0; }
  unsigned char previous_char = 0x0;
  unsigned char current_char = (unsigned char)toupper( *sequence );
  size_t length = 0;
  while( current_char )
  {
    if( current_char >= previous_char ){ length += 1; }
    else { return length; }
    previous_char = current_char;
    ++sequence;
    current_char = (unsigned char)toupper( *sequence );
  }
  return 1;
}

void get_longest_alpha_sequence( const char *sequence, char *result )
{
  if( !sequence || !result ){ return; }
  size_t longest_subsequence_length = 0;
  const char *start_longest_subsequence = sequence;
  while( *sequence )
  {
    size_t subsequence_length = step_subsequence( sequence );
    if( subsequence_length > longest_subsequence_length )
    {
      longest_subsequence_length = subsequence_length;
      start_longest_subsequence = sequence;
    }
    sequence += subsequence_length;
  }
  memcpy( result, start_longest_subsequence, longest_subsequence_length );
  result[longest_subsequence_length] = 0;
}

int main( int argc, char **argv )
{
  if( argc == 2 )
  {
    char result[strlen(argv[1])];
    get_longest_alpha_sequence( argv[1], result );
    puts( result );
  }
  else
  {
    char data[] = "dehnijwdfbre4oto4tmpfe3Taacejkzöä|REHY$%edft3G|TE$HTSATSSERgdg";
    char result[sizeof(data)] = { 0 };
    get_longest_alpha_sequence( data, result );
    puts( result );
  }
  return 0;
}

Wenn ich die nicht-alphabetischen Zeichen gemäß ASCII Tabelle vergleiche und die alphatischen nach dem Alphabet (Duh), scheint die Ausgabe zu stimmen.

Oder gibt es noch einen anderen Ansatz? Ich hatte über eine Vergleichsimplementation in einer anderen Sprache nachgedacht, aber das setzt voraus, dass ich diese in der anderen Sprache ja ebenfalls korrekt implementiert habe.