HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++ Super Simple HTML Tag Generating Code v0.1

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpletagsupergeneratingcodehtml

Problem

I made this simple code which makes easy HTML tag generation. It is open-source and right now only makes tag names and replaces tags... :/

#include 
#include 

std::string makeHtmlTags(std::string tagname, std::string tagcontent) {
    return "" + tagcontent + "";
}

void changeHtmlTag(std::string * tag, std::string newtagname)
{
    if(tag->at(2) != '/')
        *tag = "";
    else
        *tag = "";
}

int main()
{
    std::string html = makeHtmlTags("hello", "world");
    std::string tag1 = html.substr(2, 5);
    changeHtmlTag(&tag1, "goodbye");
    changeHtmlTag(&tag1, "/goodbye");
    std::cout << tag1;
}


What can I do to make it more useful?

Solution

At least in my opinion, there's quite a bit of room for improvement here. As far as usability goes, probably the most obvious change would be to directly support nesting. HTML and XML are frequently nested fairly deeply, but this doesn't really do anything to support nesting at all.

Looking at the code itself, I'd consider at least a couple of points:

1) You typically want to pass strings by const reference rather than value to avoid copying the string (unless you really need a copy, but you don't here). So:

std::string makeHtmlTags(std::string tagname, std::string tagcontent)


would change to:

std::string makeHtmlTags(std::string const &tagname, std::string const &tagcontent)


I would probably write ChangeHtmlTag to return the new string instead of modifying the existing one:

std::string changeHtmlTag(std::string const &tag, std::string const &newtagname)
{
    if(tag.at(2) != '/')
        return "";
    return "";
}


Rather than using .at, I'd also consider just returning the input intact if the input is too short to be a tag, then do the modification if it's long enough to be a tag.

The biggest problem I see, however, is that the code just doesn't seem like it does enough/has enough intelligence to make it obvious that using it gains much over playing with the strings directly.

Code Snippets

std::string makeHtmlTags(std::string tagname, std::string tagcontent)
std::string makeHtmlTags(std::string const &tagname, std::string const &tagcontent)
std::string changeHtmlTag(std::string const &tag, std::string const &newtagname)
{
    if(tag.at(2) != '/')
        return "<" + newtagname + ">";
    return "</" + newtagname + ">";
}

Context

StackExchange Code Review Q#115956, answer score: 4

Revisions (0)

No revisions yet.