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

Question

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 <iostream>
#include <fstream>

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

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

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?

Answer 1

OOP(s)!

Right now, the meat of your code is two separate functions, just floating around and minding their own business.

It would be better practice to put these two functions into a utility class. That way, they both exist together and can be easily accessed from the same parent.

I think these methods should be static.

EDIT:

According to Jerry Coffin,

These don't look like member functions to me. In any case, a class that contains only static functions should usually be a namespace instead. – Jerry Coffin

---

Versatility

It is very, very uncommon for an HTML tag to just be a tag along with inner content; there are almost always attributes.

Right now, your code can only generate something like this:

 <foo>bar</foo>

What about this?

<foo spam="eggs">bar</foo>

To improve you code, I recommend create a class/struct that stores the attribute name and value, like this:

typedef struct {
    std::string name;
    std::string val;
} HTML Attribute

Then, your method could take an array of these and loop through them, adding them to the string each time.

Answer 2

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 "<" + newtagname + ">";
    return "</" + newtagname + ">";
}

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.