C++ Class Design

Question

Currently i'm developing most of my C++ Classes with the following structure and i was wondering if the community could give me some tips on how to improve my class design to make it more portable, friendly, and maintainable in the future.

#include <vector>
#include <iostream>

namespace CompanyName
{
    class Uri;
    class Regex;

    class String
    {
    public:
        String();
        String(const char*);
        String(const String&);

        virtual ~String();

        void trim();
        void erase();
        void remove(const int);
        void remove(const int, const size_t);

        void uppercase();
        void lowercase();

        bool is_null() const;
        bool is_empty() const;

        size_t length() const;

        void append(const String&);            

        bool compare(const String&) const;
        bool compare(const String&, const bool) const;

        void replace(const Regex&, const String&);

        std::vector<String> split(const Regex&) const;

        static const char* to_utf8(const String&);
        static const uint16_t* to_utf16(const String&);
        static const uint32_t* to_utf32(const String&);

        static String from_utf8(const char*);
        static String from_utf16(const uint16_t*);
        static String from_utf32(const uint32_t*);

        static String resource(const Uri&, const int);
        static String resource(const Uri&, const String&);

        String& operator=(String rhs);
        String& operator+(const String& rhs);
        String& operator+=(const String& rhs);

        bool operator==(const String&) const;
        bool operator!=(const String&) const;

        bool operator<(const String&) const;
        bool operator>(const String&) const;

        friend std::ostream& operator<<(std::ostream&, const String&);
        friend std::istream& operator>>(std::istream&, const String&);

        static const String null;
        static const String empty;

    protected:
        struct protected_pimpl;
        protected_pimpl* _protected_pimpl;

    private:
        struct private_pimpl;
        private_pimpl* _private_pimpl;
    };

    //we have to extract the content so as to not expose the UnicodeString.
    inline std::ostream& operator<<(std::ostream& stream, const CompanyName::String& rhs)
    {
        const char* value = String::to_utf8(rhs);
        stream << value;
        delete value;
        return stream;
    }

    inline std::istream& operator>>(std::istream& stream, const CompanyName::String& rhs)
    {
        const char* value = String::to_utf8(rhs);
        stream >> value;
        delete value;
        return stream;
    }
}

`std::string` already exists. To make your class more maintainable, reusable and friendly in the future, remove it and use `std::string` instead. — Lightness Races in Orbit, Apr 06 '11 at 09:26
Thanks for your reply, I've chosen this class because everyone will be familiar with it. I'd like to stay away from debating its use and more on the original question. Thanks again for your time. — Ben Crowhurst, Apr 06 '11 at 09:30
@BenCrowhurst: Its use is _core_ to its design. Everyone is more familiar with `std::string` than they are with `CompanyName::String`, I guarantee you. — Lightness Races in Orbit, Apr 06 '11 at 09:31
Though I think you'll be more happy on [Codereview.SE](http://codereview.stackexchange.com). — Xeo, Apr 06 '11 at 09:33
@Ben: Likewise you would like to define i/o streams, other data structures, algorithms etc provided by the standard library? — Nawaz, Apr 06 '11 at 09:35
@TomalakGeret'Kal Please let it go, i'm asking about good class design, if you have nothing constructive to add please refrain from filling this space with static. Thanks. — Ben Crowhurst, Apr 06 '11 at 09:35
@BenCrowhurst: I'm sorry you feel that way. Just because you do not like my advice does not mean that it is not constructive, or that it is "static". Good luck. — Lightness Races in Orbit, Apr 06 '11 at 09:36
Please understand i agree with where your coming from and if it will please you i'll change it to another class. Thanks for your time. — Ben Crowhurst, Apr 06 '11 at 09:37
@TomalakGeret'Kal While the advice is good and valid (I don't disagree) I'm not sure it's worth making assumptions about why @BenCrowhurst has chosen this path. There may be perfectly good reasons why this class definition is described as is; although reading it (and without meta information) I'd be hard pushed myself to tell you what that is! :) — Paul McCabe, Apr 06 '11 at 19:44

score 1 · Answer 1 · answered Apr 06 '11 at 09:37

1

You have no virtual member functions, so your class is clearly not meant to be used polymorphically. Thus, making the destructor virtual is unnecessary (and causes the class to have an unnecessary v-table) and there is no need for a protected pimpl.

Also, whenever possible, always prefer free functions (declared as friends if necessary) for operators instead of member functions.

answered Apr 06 '11 at 09:37

ildjarn

62,044
9
127
211

I'm curious, why do you recomment friend operators instead of member operators? – DevSolar Apr 06 '11 at 09:40
Some people recommend _always_ making your class polymorphism-safe, "just in case" somebody decides to use it that way. I wouldn't (though I'm hard-pressed to justify that, tbqh). – Lightness Races in Orbit Apr 06 '11 at 22:07
@DevSolar: http://stackoverflow.com/questions/4622330/operator-overloading-member-function-vs-non-member-function/4622467#4622467 – Lightness Races in Orbit Apr 06 '11 at 22:09

score 1 · Accepted Answer · answered Apr 06 '11 at 09:51

I would say that the first, best step would be to remove everything you don't need.

Consider the advice of Monoliths Unstrung: classes are more maintainable and 'future-proof' when they are pruned down to their primary concepts.

For example: why does replace() need to be a member function? Or the to/from_utfX() functions?

C++ Class Design

2 Answers2