libhtmlpp rc1

Tuxist1

Moin,

habe eine Bibliothek kurz vor der Fertigstellung.
https://tuxist.de/git/jan.koester/libhtmlpp

Für Anregungen oder Verbesserungsvorschläge bin ich gern zu haben.

Lizenz: BSD

Mit der Bibliothek kann man HTML parsen und Manipulieren setzte hauptsächlich in Kombination mit libhttppp ein aber CGI ist auch möglich.

Quiche Lorraine

@Tuxist1

Folgende Punkte fallen mit auf:

Du nutzt verdächtig oft friend. Ein Anzeichen dass dein Design nicht stimmt.
Du nutzt goto? Ernsthaft? Das mündet schnell in Spagetti-Code und mach Abläufe schwer nachvollziehbar. Raus damit! Ist vollkommen unnötig und fehleranfällig.
In utils.h nutzt du auch noch manuelle Speicherverwaltung. Raus damit! Ist vollkommen unnötig und fehleranfällig.
Die Klasse Element ist meines Erachtens ein Verstoß gegen Single Responsible Prinziple, da dies eine intrinsche Liste zu sein scheint. Warum nutzt du kein entsprechende STL Container mit std::variant?
CppCheck motzt dementsprechend oft mit Potenial invalid cast. Du castest relativ oft von der Basisklasse auf die obere Klasse. Wie kannst du sicher sein dass das funktioniert?
Kannst du mir mal folgenden Code erklären?

void libhtmlpp::Element::insertBefore(libhtmlpp::Element* el){
    std::unique_ptr<Element> nel;
    switch(el->getType()){
        case HtmlEl:{
            nel=std::make_unique<HtmlElement>();
            break;
        }
        case TextEl:{
            nel=std::make_unique<TextElement>();
            break;
        }
        case CommentEl:{
            nel=std::make_unique<CommentElement>();
            break;
        }
        case ScriptEL:{
            nel=std::make_unique<ScriptElement>();
            break;
        }
    }
    _copy(nel.get(),el);
    std::unique_ptr<Element> prev=std::move(_prevElement->_nextElement);
    _prevElement->_nextElement=std::move(nel);
    nel->_nextElement=std::move(prev);
}

Übrigens motzt hier CppCheck rum, da du nel erst movst und danach drauf zugreifst.

Versuche mal deutlich weniger Zeiger zu nutzen. Du brauchst diese nur in seltenen Fällen.

Tuxist1

Funktionieren tut es anscheinend meine Blog Software nutzt die Bibliothek ^^ .
Mit utils.h haste recht habe vergessen die beiden alten Funktionen zu löschen.

    _copy(nel.get(),el); //Kopieren das Baumes
    std::unique_ptr<Element> prev=std::move(_prevElement->_nextElement); //ensprechen besitztümer des pointers ändern
    _prevElement->_nextElement=std::move(nel);  //nochmal für das nachfolgende element
    nel->_nextElement=std::move(prev); // ach merke doppelt gemoppelt ändere ich

DocShoe

Hab's nur kurz überflogen...

führende Underscores sollten vermieden werden, in den meisten Fällen sind sie Library Implementors vorbehalten.
deine Funktionen sind teilweise recht lang und machen mehrere Dinge. Du könntest dir überlegen, wie du Funktionen mit mehr als 80 Zeilen in mehrere Funktionen aufteilen könntest.
leere Konstruktoren und Destruktoren mit default kennzeichnen oder ganz weglassen
statische char[] Konstanten durch std::string_view ersetzen
Elemente bereits im Header initialisieren

Tuxist1

habe angefangen es kleine Methoden zu packen die Methode Buildtree ist jetzt in mehrere aufgeteilt
Konstruktoren habe ich paar auf default gesetzt das geht aber nicht wenn dieser den Konstruktor der vererbten klasse mit argument aufruft richtig ?
-char habe jetzt durch array mit string_View ersetzt
-header muss ich jetzt noch ran

Th69

Du hast noch ein paar Schreibfehler:

in HtmlElement:
- Get(Int)Atributte -> Get(Int)Attribute
- _serialelize-> _serialize

Und da du in html.cpp schon std::isspace verwendest? Warum hast du dann die Funktionen in utils.h selber implementiert (und dann noch inperformant mittels Schleife)? Es gibt doch std::isdigit und std::isalpha.
Und wenn du, unabhängig von der aktuellen locale immer nur die 26 Klein-und Großbuchstaben erkennen willst (und keine Umlaute o.ä.), dann gibt es noch std::isalpha(std::locale) (mit std::locale("C")).

Und in der Funktion _buildTree solltest du ebenfalls std::tolower (anstatt dein ascii_tolower) verwenden.

PS: Du hast deine Änderungen (bzgl. _BuildTree) aber noch nicht eingecheckt, oder?

Quiche Lorraine

@Tuxist1

Was hältst du von folgendem Code?

#include <variant> 
#include <list>
#include <vector>

// Beispieltypen


class HtmlElement
{

};

class TextElement
{

};

class CommentElement
{

};

// ....

using Element = std::variant<HtmlElement, TextElement, CommentElement>;  

/**
 * @brief Definition eines HTML Kommandos
 * 
 * Ein Kommando besteht immer aus einem HTML Befehl und weiteren Unterbefehlen.
*/
class HtmlCommand
{
public:
    std::vector<HtmlCommand> mSubCmds; 
    Element mElement;                   // Evt. anderer Name wählen?
};


class HtmlDocument
{
public:
    std::vector<HtmlCommand> mCommands;
};



int main()
{
    return 0;
}