Issues with strings

Question

I have a snippet of code that takes the output of another function (that function gets user input) and replaces all spaces with dashes. Or, rather, that's what it's supposed to do. Instead, it takes the first word of the string and ignores the rest (e.g. 'Hello World' -> 'Hello'). Here is the snippet:

void info::name(string* name, string title){
    char arr[title.size() + 1];
    strcpy(arr, title.c_str());
    int o = 0;
    while(arr[o] != 0){
        if(arr[o] == ' '){
            arr[o] = '-';
        };
        o++;
    };
    *name = arr;

Is there any reason why this wouldn't work?

EDIT: What do you mean by combining C-style arrays and std::string?

EDIT2: I tried using std::replace, but the same thing happens.

EDIT3: I can't get getline() to work. Here is how I'm using it:

getline(cin, *title, "/n");

Why is this not working?

FINAL_EDIT: I finally got it to work! Here is what worked for me:

void info::title(string* title){
    cout << "Enter the name of your mod: ";
    getline(cin, *title); cout << endl;}
void info::name(string* name, string title){
    replace(title.begin(), title.end(), ' ', '-');
    *name = title;}

Once again, thanks, all!

there are a lot of reasons why this wouldn't work actually!! — Jean-François Fabre, Sep 09 '16 at 15:08
@Creep2DJ This declaration char arr[title.size() + 1]; is invalid in C++ .:) — Vlad from Moscow, Sep 09 '16 at 15:08
TL;DR [`std::replace`](http://en.cppreference.com/w/cpp/algorithm/replace). — juanchopanza, Sep 09 '16 at 15:08
@VladfromMoscow I did that this morning and it worked on C++. Not in C I agree. you can define dynamic-sized arrays on the stack all right in C++. — Jean-François Fabre, Sep 09 '16 at 15:09
@Jean-FrançoisFabre No, Variable length arrays are illegal in c++, it will work in c though. — Fantastic Mr Fox, Sep 09 '16 at 15:10
@Creep2DJ Also check whether title is indeed contains moew than one word. Perhaps you entered data using the operator >> instead of the function getline. — Vlad from Moscow, Sep 09 '16 at 15:10
You should do some debugging (or just follow @juanchopanza's suggestion). — Oliver Charlesworth, Sep 09 '16 at 15:11
@Ben: compiled a lot of them using g++ 4.9.x, Worked fine.Should fail with compilation error? I thought that was the contrary: can't do in C. — Jean-François Fabre, Sep 09 '16 at 15:11
Why, oh why are you mixing C-style strings, and `std::string`. I feel like jumping off a cliff reading this code. — Arnav Borborah, Sep 09 '16 at 15:11
@Jean-FrançoisFabre http://stackoverflow.com/questions/1887097/variable-length-arrays-in-c — Fantastic Mr Fox, Sep 09 '16 at 15:11
@Ben: could it be that g++ doesn't care of what is legal and accepts it since it technically can accept it? (ok we should use vectors, but what if the code is already crap and we try to fix it cheaply ? :)) — Jean-François Fabre, Sep 09 '16 at 15:13
@Jean-FrançoisFabre But it is technically incorrect: The C++ standard states that array size must be a constant expression (8.3.4.1). Are you sure in the compilations you did you were not using say `sizeof(someType)` in the array or similar? — Fantastic Mr Fox, Sep 09 '16 at 15:15
*EDIT: What do you mean by combining C-style arrays and std::string?* -- So to the OP: Do you want an actual C++ solution, or stick with `C` code? In `C++`, the real solution is to use (already mentioned) `std::replace` or some other algorithm(s), and just drop using the code you've written. — PaulMcKenzie, Sep 09 '16 at 15:21
@Jean-FrançoisFabre `-pedantic -Werror` or it doesn't count. — n. m. could be an AI, Sep 09 '16 at 15:28
One reason why it wouldn't work is you replacing all the spaces with dashes in the variable `arr`, and then forget about it completely, throw `arr` away and never use it. There might be other reasons but this one is pretty big. You probably would catch it in less than a minute if you used a debugger. — n. m. could be an AI, Sep 09 '16 at 15:32
@n.m. I didn't c/p it in, this was an error in this snippet only. — , Sep 09 '16 at 18:33

Vlad from Moscow · Accepted Answer · 2016-09-09T16:43:19.120

Here is a demonstrative program that shows how the effect you want can be achieved.

#include <iostream>
#include <string>
#include <algorithm>
#include <iterator>

struct A
{
    static std::string name( const std::string &title )
    {
        std::string result;
        result.reserve( title.size() );

        std::replace_copy_if( title.begin(), title.end(),
                              std::back_inserter( result ),
                              []( char c ) { return c == ' '; }, '-' );

        return result;                            
    }
};

int main() 
{
    std::string title;
    std::string name;

    std::cout << "Enter a title: ";

    if ( std::getline( std::cin, title ) ) name = A::name( title );

    std::cout << name << std::endl;

    return 0;
}

If to enter string "Hello World"

then the output will look like

Enter a title: Hello World
Hello-World

I used the standard algorithm std::replace_copy_if only to show how to use lambda expressions. For example you could substitute also the tab character for the dash.

std::replace_copy_if( title.begin(), title.end(),
                      std::back_inserter( result ),
                      []( char c ) { return c == ' ' || c == '\t'; }, '-' );

Otherwise you can use standard algorithm std::replace_copy the following way

std::replace_copy( title.begin(), title.end(),
                   std::back_inserter( result ),
                   ' ', '-' );

As you see the task can be done without using an intermediate array. Moreover this declaration of an array

char arr[title.size() + 1];

is not C++ compliant declaration though some compilers can have their own extensions that allow such declarations..

Also there is a typo in your function. I think you mean

*name = arr;

instead of

*name = title;

that's -1 for replying to this bad question, +2 for showing off on C++ STL stuff & other subtleties I did not know :) — Jean-François Fabre, Sep 09 '16 at 16:29
How come you're using a variable address in the function definition instead of a pointer? — , Sep 09 '16 at 17:06
@Creep2DJ -- That is not an address. The `&` in this context means `reference`. — PaulMcKenzie, Sep 09 '16 at 19:30

score 0 · Answer 2 · answered Sep 09 '16 at 15:40

Is there any reason why this wouldn't work?

Yes you create temporary array, modify it and then assign unmodified original string to string pointed by first argument.

You do not need temporary array at all and passing a pointer is not a C++ way to do it:

std::string info::name( std::string title )
{
    std::replace( title.begin(), title.end(), ' ', '-' );
    return title;
}

if you do want to use loop (required by homework etc) just do it on string itself:

std::string info::name( std::string title )
{
    for( size_t i = 0; i < title.length(); ++i ) {
        if( title[i] == ' ' )
            title[i] = '-';
    }
    return title;
}

Issues with strings

2 Answers2