2

I just started learning boost shared pointers.

I wrote a short program, results look good but I'm not sure if memory is deallocating well with my code. I would like to ask, if someone could look at my code and tell if I correctly use shared pointers.

#include <boost/shared_ptr.hpp>
#include <iostream>
#include <vector>
#include <string>

#define VECTSIZE 10

typedef boost::shared_ptr<std::string> StringPtr;
typedef std::vector<StringPtr> StringVect;

///////////////////////////////////////////////////////////////

std::string random_string (size_t length);

///////////////////////////////////////////////////////////////

int main()
{
    StringVect vect;

    for (int i = 0; i < VECTSIZE; i++)
    {
        std::string * stdstr;
        stdstr = new std::string;
        *stdstr = random_string(10);
        std::cout << *stdstr << "\r\n";

        StringPtr str(stdstr);
        vect.push_back(str);
    }

    std::cout << "\r\n\r\n";

    for (int i = 0; i < VECTSIZE; i++)
    {
        std::cout << *vect[i] << "\r\n";
    }

    vect.clear();
    system("pause");
    return 0;
}

///////////////////////////////////////////////////////////////

std::string random_string (size_t length)
{
    auto randchar = []() -> char
    {
        const char charset[] =
        "0123456789"
        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
        "abcdefghijklmnopqrstuvwxyz";
        const size_t max_index = (sizeof(charset) - 1);
        return charset[ rand() % max_index ];
    };

    std::string str(length,0);
    std::generate_n( str.begin(), length, randchar );
    return str;
}

Thanks for any advice; I hope it'll be helpful for me and others.

Rody Oldenhuis
  • 37,726
  • 7
  • 50
  • 96
ST3
  • 8,826
  • 3
  • 68
  • 92
  • 1
    Not directly related to the question, but seeing as you already have C++11 support (`auto`, lambdas), you can as well use `std::shared_ptr` instead of the Boost variant. – Angew is no longer proud of SO Jun 05 '13 at 06:38
  • Shared pointers are new thing for me so anyway std or boost would be tricky. What is more std::shared_ptr are suported on WIN XP SP 3 and latter. That means, what it wouldn't work on some PC. – ST3 Jun 05 '13 at 06:42
  • even better are `std::unique_ptr`s in a `std::vector`. You can move them into the vector. The reason for this is that `shared_ptr`s are a bit heavy. – user1095108 Jun 05 '13 at 06:52
  • `std::shared_ptr` is a pure template class, making it a compile-time construct. It requires VS 2010 or later, but it does not depend on the target system. – Angew is no longer proud of SO Jun 05 '13 at 06:53
  • *"someone could look at my code and tell if I correctly use shared pointers."* - No, you're not, you're using `boost::shared_ptr`s in a case where a mere `std::string` is more appropriate. Keep in mind that `shared_ptr`s are not meant as a replacement for proper value semantics, this is C++ and not Java. Smart pointers only free you from manual memory management considerations if those considerations would actually be existent otherwise. – Christian Rau Jun 05 '13 at 09:14

2 Answers2

4

Your use is correct in the sense that there are no direct memory leaks. However, you're not really exception safe - if random_string throws, you'll leak stdstr. It's better (and more idiomatic) to bypass rwa pointers entirely. Here's an example with using std::shared_ptr:

for (int i = 0; i < VECTSIZE; i++)
{
    StringPtr str = std::make_shared<std::string>();  // Encapsulates new
    *str = random_string(10);
    std::cout << *str << '\n'; //No \r here: text streams insert it on Windows automatically

    vect.push_back(str);
}

Also, as @ForEveR noted, there's little reason to allocate std::string dynamically in real world apps. But I assume you use it just as an excercise with smart pointers, which is fine of course.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
3

All is okay, but you needn't vect.clear() string. However, string is value-type, don't use shared_ptr of string.

ForEveR
  • 55,233
  • 2
  • 119
  • 133
  • Also I would change the signature of random_string to void randomstring(size_t length, std::string& out) for memory and speed optimizations (of course in this example it isn't something to worry about!) – Kourosh Jun 05 '13 at 06:39
  • 3
    @Kourosh No. Look up Named Return Value Optimization. Don't do this. – Yuushi Jun 05 '13 at 06:43
  • 1
    @Kourosh With C++11's move semantics, the OP's signature is probably preferable. Also, [want speed? Pass by value.](http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/) – Angew is no longer proud of SO Jun 05 '13 at 06:48