Why does my function not switch the first character with the last one of my string?

Viewed 79

I picked up a challenge on r/dailyprogrammer on reddit which wants me to match a necklace and put the last letter at the beginning of a string. I've considered using nested for loops for this but this has made me really confused.
Instead I chose the way of replacing the last with the first character in an if-statement. But I am not getting my desired output with it, though I've tried everything what comes into my mind.
I used even std::swap() which didn't lead me to success either.

Here's the code:

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

using namespace std;

string same_necklace(string& sInput, string& sOutput)
{
    for (string::size_type i = 0; i < sInput.size(); i++)
    {
        if (sInput[i] == sInput.size())
        {
            sInput[0] = sInput[sInput.size()];
        }
    }

    for (string::size_type j = 0; j < sOutput.size(); j++)
    {
        if (sOutput[j] == sOutput.size() - 1)
        {
            sOutput[0] = sOutput[sOutput.size()];
        }
    }

    return sInput, sOutput;
}

int main()
{
    system("color 2");

    string sName{ "" };
    string sExpectedOutput{ "" };

    cout << "Enter a name: ";
    cin >> sName;
    cout << "Enter expected output: ";
    cin >> sExpectedOutput;


    cout << "Result: " << same_necklace(sName , sExpectedOutput) << endl;

    return 0;
}

And of course the link to my challenge (don't worry, it's just Reddit!): https://www.reddit.com/r/dailyprogrammer/comments/ffxabb/20200309_challenge_383_easy_necklace_matching/

While I am waiting (hopefully) for a nice response, I will keep on trying to solve my problem.

2 Answers

In your if you compare the value of the current index (inside the loop) with the size of the string. Those are two unrelated things.
Also, you use a loop though you only want to do something on a single, previously known index.

for (string::size_type i = 0; i < sInput.size(); i++)
{
    if (sInput[i] == sInput.size())
    {
        sInput[0] = sInput[sInput.size()];
    }
}

You could change the if condition like this to achieve your goal:

if (i == sInput.size()-1) /* size as the index is one too high to be legal */

But what is sufficient and more elegant is to drop the if and the loop. completely

/* no loop for (string::size_type i = 0; i < sInput.size(); i++)
{ */
    /* no if (sInput[i] == sInput.size())
    {*/
        sInput[0] = sInput[sInput.size()-1]; /* fix the index*/
    /* }
} */

I.e.

sInput[0] = sInput[sInput.size()-1]; /* fix the index*/

Same for he output, though you got the correct index already correct there.

This is not intended to solve the challenge which you linked externally,
if you want that you need to describe the challenge completely and directly here.
I.e. this only fixes your code, according to the desription you provide here in the body of your question,
"put the last letter at the beginning of a string".
It does not "switch" or swap first and last. If you want that please find the code you recently wrote (surely, during your quest for learning programming) which swaps the value of two variables. Adapt that code to the two indexes (first and last, 0 and size-1) and it will do the swapping.

So much for the loops and ifs, but there is more wrong in your code.

This

return sInput, sOutput;

does not do what you expect. Read up on the , operator, the comma-operator.
Its result is the second of the two expressions, while the first one is only valuated for side effects.
This means that this

cout << "Result: " << same_necklace(sName , sExpectedOutput) << endl;

will only output the modified sExpectedOutput.
If you want to output both, the modified input and the modified output, then you can simply

cout << "Result: " << sName << " " << sExpectedOutput << endl;

because both have been given as reference to the function and hence both contain the changes the function made.

This also might not answer the challenge, but it explains your misunderstandings and you will be able to adapt to the challenge now.

You have not understand the problem i guess. Here you need to compare two strings that can be made from neckless characters.

Lets say you have neckless four latters word is nose. Combination is possible

1)nose 2)osen 3)seno 4)enos

your function (same_necklace) should be able to tell that these strings are belongs to same necklace

if you give any two strings as inputs to your function same_necklace your function should return true.

if you give one input string from above group and second input string from other random word thats not belongs to above group, your function should return false.

In that sense, you just take your first string as neckless string and compare other string with all possible combination of first string.

just move move you first latter of first input string to end and then compare each resulting string to second input string.

below is the function which you can use

void swap_character(string &test)
{
    int length = test.length(); 
    test.insert(length, 1, test[0]); 
    test.erase(0, 1);   
}
Related