-2

I was trying to solve a problem from codewars and it seems like I have a problem with my output.

This is the problem I am trying to solve

This is my output and my code

The code works when I run it on codeblocks. There is a missing " in the output and I think that is due to the NULL character that I am adding at the end of the string. My code is not meant to output any quotation marks, and I think that the NULL character interferes with the website's code, erasing their last quotation mark. Can anybody help me? Thank you! This is the code:

#include <string.h>

std::string to_camel_case(std::string a)
{
    int q = a.size();
    for (int i = 0; i < q; i++)
        if (a[i] == '-' || a[i] == '_') {
            if (a[0] > 96 && a[0] < 123)
                a[i + 1] = a[i + 1] - 32;
            for (int j = i; j < q - 1; j++) {
                a[j] = a[j + 1];
            }
            a[q - 1] = 0;
        }
    return a;
}
drescherjm
  • 10,365
  • 5
  • 44
  • 64
  • 1
    Welcome to Stack Overflow. Please read the [About](http://stackoverflow.com/tour) page soon and also visit the links describing [How to Ask a Question](http://stackoverflow.com/questions/how-to-ask) and [How to create a Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) (MCVE). Providing the necessary details, including your MCVE, compiler warnings and associated errors, if any, will allow everyone here to help you with your question. (also, never post pictures of code, copy and past into your question, indented by 4-spaces, or wrapped in `\`\`\`` to format) – David C. Rankin Feb 26 '21 at 01:51
  • https://www.techiedelight.com/iterate-over-characters-string-cpp/ check method 2, and if you want to press on, method 3 because iterating over stuff is gonna happen a lot and it's good to do it in a modern way – szpanczyk Feb 26 '21 at 02:09
  • also, please fix your indentation, it makes a world of difference when reading somebody elses code – szpanczyk Feb 26 '21 at 02:10
  • if(a[0]>96 && a[0]<123) I would suggest using specific character literals here for readability also – szpanczyk Feb 26 '21 at 02:11
  • a[q - 1] = 0; <-- q never changes, you're doing this every iteration of the for loop. – szpanczyk Feb 26 '21 at 02:14
  • I think that above a[q-1]=0 is intentional since the letters are being moved left, though I wouldn't normally do that myself. – AhiyaHiya Feb 26 '21 at 02:17
  • Sure it is intentional, but first even if it did anything, it wouldn't be necessary to do it during string processing when it could be done after the for loop ends, once. But again, as I said, q never changes and nothing is ever written to a[q -1] except for 0, over and over again. I am no expert on compiler optimization, but I think it actually is optimized out of the loop. – szpanczyk Feb 26 '21 at 02:20
  • a[j] = a[j + 1]; // this is accessing a[q]. Baaad. – szpanczyk Feb 26 '21 at 02:29
  • 2
    The code you've shown doesn't need `string.h` - it doesn't even need the proper header (`cstring`). It does however need the C++ header `string`.. Please present a [mre]. – Ted Lyngmo Feb 26 '21 at 02:30
  • Oh my bad, j only goes as far as q-2 – szpanczyk Feb 26 '21 at 02:41
  • 1
    @szpanczyk Who knows what `q` is? Making a [mre] solves that. – Ted Lyngmo Feb 26 '21 at 02:49
  • OH again, not my bad - someone is fixing the code in the question. Is that a good idea anyway? I don't think so. – szpanczyk Feb 26 '21 at 02:49
  • @szpanczyk It's usually a good idea to remove small flaws since it helps people to help. – Ted Lyngmo Feb 26 '21 at 02:50
  • Not really if you take a code with errors in question and fix the errors. Then it looks like someone is asking why their working code is not working. It's a mess. – szpanczyk Feb 26 '21 at 02:51
  • @szpanczyk I agree. Is that what happened? – Ted Lyngmo Feb 26 '21 at 02:52
  • Yes, an out of bounds access was fixed, although I still haven't figured out what exactly is the problem here. It would seem OP is returning a proper null terminated string unless he segfaults. – szpanczyk Feb 26 '21 at 02:52
  • 1
    No, wait. I think I made a terrible mistake. :D a[q] seems to be where original null should be in the input right? I somehow thought we have a typical C situation, where a[q-1] is the original null. – szpanczyk Feb 26 '21 at 02:55
  • The way I see it, while I see I think an obvious error in the algorithm that will reveal itself when OP resolves this problem first... Otherwise it should produce proper conversion with trailing nulls all the way. – szpanczyk Feb 26 '21 at 03:05
  • I used a count variabel that keeps track of how many "_" and "-" I removed and at the end I resized my vector like this: a.resize(q-count). q is equal to the initial size of a, im sorry if it wasn't clear. – Cristyantm97 Feb 26 '21 at 15:22

2 Answers2

0

Have you tried resizing the string before returning it?

std::string to_camel_case(std::string a)
{
  int q = a.size();
  int count = 0;
  for (int i = 0; i < q; i++)
  {
    if (a[i] == '-' || a[i] == '_')
    {
      if (a[0] > 96 && a[0] < 123)
        a[i + 1] = a[i + 1] - 32;

      for (int j = i; j < q - 1; j++)
        a[j] = a[j + 1];

      // Keep track of the amount to remove
      count++;
    }
  }

  if (count)
    a.resize(q - count);

  return a;
}
AhiyaHiya
  • 755
  • 3
  • 13
  • I think that was the problem, It fixed that issue but now I get another one and I dont know what is the problem with my code. With the exact same code as you i recieve this error message: "Expected: equal to "APippiIsPippi" Actual: "APippiispippi", unfortunately I dont know what was the input given. – Cristyantm97 Feb 26 '21 at 15:07
0

It seems like you're making it way more complicated than it needs to be which is causing simple mistakes to be overlooked. Modern C++ practices tries to alleviate these concerns by doing the work for you, such as iterating over a container and erasing a specified element from that container.

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

std::string ToCamelCase(std::string str) noexcept {
    if(str.empty()) return {};
    for(auto iter = std::begin(str); iter != std::end(str) - 1; /* DO NOTHING */ ) {
        if(*iter == '-' || *iter == '_') {
            iter = str.erase(iter);
            *iter = static_cast<char>(std::toupper(*iter));
            continue;
        }
        ++iter;
    }
    return str;
}

I only wish I knew of an STL algorithm that had look-ahead capabilities. That way this could be a simple call to a std::transform-like function.

Casey
  • 10,297
  • 11
  • 59
  • 88
  • Your code works perfect, it passes all the tests, but I dont want to use it beacause I don't understand it. I learned c++ in highschool, and I dont know how "auto iter" works, im not used to using "std::" we used "using namespace std" in highschool. Also I dont really know how to work with strings because we were declaring them like this "char string[20]". And functions for this type o declaration are different. I really apreciate your effort to write this code for me, thank you! – Cristyantm97 Feb 26 '21 at 15:03
  • @Cristyantm97 ....You had a pretty bad teacher. They were obviously teaching C as C++. – Casey Feb 26 '21 at 15:46
  • @Cristyantm97 see also [why using namespace std is a bad practice](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – Casey Feb 26 '21 at 15:50
  • @Cristyantm97 If this answered solved your problem, please mark it as accepted by clicking the checkmark. – Casey Feb 26 '21 at 15:52