C++ segmentation fault in function

Question

Learning some basic C++, trying to understand functions but having a hell of a time trying to properly write the function. Here's the original code (works fine):

#include <iostream>
#include <string>
//#include "assn.h"

using namespace std;

int main()
{
    string userinput;
    char ch1 = '-';

    cout << "Enter word: ";
    getline(cin, userinput);

    int i = 0;
    for (i; i < userinput.size(); ++i)
    {
        if (userinput.at(i) >= 'a' && userinput.at(i) <= 'z')
        userinput.at(i) = ch1;
    }

    cout << userinput << endl;
    return 0;
}

and here's the code I typed trying to make it a function:

#include <iostream>
#include <string>
//#include "assn.h"

using namespace std;

string inputUnsolved(string input);

int main()
{
        string userinput;
        cout << "Enter word: ";
        getline(cin, userinput);

        inputUnsolved(userinput);

        cout << userinput << endl;
        return 0;
}

string inputUnsolved(string input)
{
    char ch1 = '-';

    int i = 0;
    for (i; i < input.size(); ++i)
    {
        if (input.at(i) >= 'a' && input.at(i) <= 'z')
            input.at(i) = ch1;
    }
}

It compiles fine, but after entering the userinput and trying to execute, it displays "Segmentation fault"

Tried rewriting a few times with no luck and I quite honestly don't know enough to find a way to fix it. The code basically reads in a string variable and displays it as a set of dashes (hangman).

Answer 1

Pass by reference instead

void inputUnsolved(string& input);

Answer 2

You pass the parameter input by value to the function. That means that it gets a copy of the variable in main. Any changes is performed on this copy only, and does not affects the original value.

Try passing by reference instead, string&.

Also, the function has a return type string, but I don't see any return statement in the function. Either return some value or change the return type to void.

Answer 3

Try this:

#include <iostream>
#include <string>
//#include "assn.h"

using namespace std;

void inputUnsolved(string &input);

int main()
{
        string userinput;
        cout << "Enter word: ";
        getline(cin, userinput);
        inputUnsolved(userinput);
        cout << userinput << endl;
        return 0;
}

void inputUnsolved(string &input)
{
   char ch1 = '-';
   for (int i=0; i<input.size(); ++i)
   {
      if (input.at(i) >= 'a' && input.at(i) <= 'z')
         input.at(i) = ch1;
   }
}

First there is no need to return a string. Second, as said in an answer you should send the string by reference so the changes are reflected in the input string.

If you want to return a string, you can do this:

string inputUnsolved(string input)
{
   char ch1 = '-';
   for (int i=0; i<input.size(); ++i)
   {
      if (input.at(i) >= 'a' && input.at(i) <= 'z')
         input.at(i) = ch1;
   }
   return input;
}

The c++ compiler should optimize it for you. Please take into account should... you never know what the compiler's devs did...

Answer 4

You are doing some wrong things:

1. you pass your string and expect to be edited which is wrong.
2. you define your function to return string which is not returning anything

You are doing some bad things:

1. using namespace std this bad practise.
2. using int for indexing which is a signed type and less than the possible size.

This is the solution to your problems:

#include <iostream>
#include <string>


std::string inputUnsolved(const std::string& input);

int main()
{
        std::string userinput;
        std::cout << "Enter word: ";
        std::getline(std::cin, userinput);

        userinput=inputUnsolved(userinput);

        std::cout << userinput << std::endl;
        return 0;
}

std::string inputUnsolved(const std::string& input)
{
    std::string result;
    result.reserve(input.size());
    char ch1 = '-';

    for (std::size_t i=0; i < input.size(); ++i)
    {
        if (input.at(i) >= 'a' && input.at(i) <= 'z'){
            result.push_back(ch1);
        }
        else{
            result.push_back(input.at(i));
        }
    }
    return  result;
}

Answer 5

Try this it will work for you for sure, the problem was that you passed the variable input by value instead of reference, because if not, no matter what you entred it will not change to "---", when you use refrence you make sure you modify the same variable entred. The second thing that i added is that you dont need to make the return value of your function a string, make it easier and make it void instead.

  #include <iostream>
    #include <string>
    //#include "assn.h"

    using namespace std;

    void inputUnsolved(string& input);

    int main()
    {
            string userinput;
            cout << "Enter word: ";
            getline(cin, userinput);

            inputUnsolved(userinput);

            cout << userinput << endl;
            return 0;
    }

    void inputUnsolved(string& input)
    {
        char ch1 = '-';

        int i = 0;
        for (i; i < input.size(); ++i)
        {
            if (input.at(i) >= 'a' && input.at(i) <= 'z')
                input.at(i) = ch1;
        }


    }