Extracting numbers before colon

I need to extract all numbers before a colon in a string, e.g.:

• abc1234:blah -> 1234

• 5n63:124 -> 63

• stringwithnocolon -> ""

• stringwith:nonumber -> ""

Limitations:

• no external dependency (looking forward to keep it simple)

• no

• not C++11

Here is the code I came up with:

const std::string DNAStorage::get_block(const std::string& name)
{
    char* buffer = new char[name.size()];
    char* temp = new char[name.size()];
    size_t i, j;
    for (i = 0; i  0; j--)
    {
        if (!isdigit(buffer[j]))
            break;

        temp[i - j - 1] = buffer[j];
    }

    temp[i - j - 1] = '\0';

    std::string result = std::string(temp);
    std::reverse(result.begin(), result.end());
    return result;
}

But I don't really like it. How could I make this code better?

Jamal's answer already covers some of this, but I wanted to provide slightly different reasons for some of his advice.

1. Return an std::string rather than a const std::string.

Returning an std::string makes your function more useful to a caller because it gives them more options.

They can do this:

std::string s1 = DNAStorage::get_block ("abc1234:blah") ;

Or this:

const std::string s1 = DNAStorage::get_block ("abc1234:blah") ;

Or even this:

const std::string &s1 = DNAStorage::get_block ("abc1234:blah") ;

Unless you are trying to force some kind of interface onto the caller, there isn't really any good reason (from a programming perspective) to return a const std::string.

1. Cut down on copying.

You copy a substring of name into buffer, then a substring of buffer into into temp, and then you copy temp into result. This is unnecessary. See section 4 of this post for an example that avoids all of this copying.

1. Avoid memory leaks and focus on exception safety.

You dynamically allocate memory for two character arrays. If you actually needed a buffer (which you don't), you could just do something like this:

std::string buffer (name.size (), ' ') ;

1. Here's an example using std::find() and iterators.

Edit: 200_success was kind enough to provide a more concise version:

std::string DNAStorage::get_block (const std::string &name)
{
    typedef std::string::const_iterator iter ;

    iter colon = std::find (name.begin (), name.end (), ':') ;

    if (colon == name.end ()) { 
        return "";
    }

    iter start = colon ;

    while (start != name.begin () && std::isdigit (*(start - 1))) { 
        --start ;
    } 

    return std::string (start, colon) ;
}

Test cases:

int main ()
{
    const std::string s1 = DNAStorage::get_block ("abc1234:blah") ;
    const std::string s2 = DNAStorage::get_block ("5n63:124") ;
    const std::string s3 = DNAStorage::get_block ("nocolon") ;
    const std::string s4 = DNAStorage::get_block ("nonumber:ffff") ;
    const std::string s5 = DNAStorage::get_block ("5363:124") ;
    const std::string s6 = DNAStorage::get_block ("v32463:124") ;

    return 0 ;
}

1. Here's an example with std::reverse_iterator.

First we have to create a unary predicate for std::find_if.

bool is_not_digit (char c)
{
    return std::isdigit (c) == 0 ;
}

And here's how we can code the function:

std::string DNAStorage::get_block (const std::string &name)
{
    typedef std::string::const_iterator const_iterator ;
    const const_iterator colon_position = std::find (name.begin (), name.end (), ':') ;

    if (colon_position == name.end ()) {
        return "" ;
    }

    std::reverse_iterator  rstart (colon_position) ; 
    std::reverse_iterator  rend = std::find_if (rstart, name.rend (), is_not_digit) ; 

    std::string out (rstart, rend) ;
    std::reverse (out.begin (), out.end ()) ;
    return out ;
}

StackExchange Code Review Q#75740, answer score: 4