-3

Here is the code I have:

string words[n];
 .
 .
 .
 for(int j = 0; j < sizeof(words)/sizeof(words[0]); j++){
            if(words[j].find(new_word) != std::string::npos){...} //abort(3) called
 } 

However, SIGABRT isn't called in the same code when I call find on a normal string, like

  for(int j = 0; j < sizeof(words)/sizeof(words[0]); j++){
            if(s.find(new_word) != std::string::npos){...} //abort(3) NOT called

Why is this behavior happening? Isn't words[j] referring to a string element? Edit: adding the entire code, in case the snippet isn't sufficient :

#include <stdio.h>
#include<string>
#include<stdlib.h>
#include<iostream>
using namespace std;
int main() {
    int T;
    cin>>T;
    while (T--){
        int n;
        cin>>n;
        string words[n];
        for(int i =0; i<n;i++){
            cin>>words[i];
        }
        string s;
        cin>>s;
        string new_word;
        //start inserting spaces now
        for(int i =1;i<=s.length();i++){
            new_word = s.substr(0,i);
            //cout<<s<<endl;
            //start scanning
            for(int j = 0; j < sizeof(words)/sizeof(words[0]); j++){
                if(words[j].find(new_word) != std::string::npos){
                    s = s.erase(s.find(new_word), new_word.length());
                }
            }
        }
        if(s.length() == 0)
            cout<<"1"<<endl;
        else
            cout<<"0"<<endl ;
    }
    return 1;
}

Edit 2: The question is here :http://practice.geeksforgeeks.org/problems/word-break/0

a3y3
  • 1,025
  • 2
  • 10
  • 34
  • 2
    Hard to say. Please, try to reproduce with an [MCVE](http://stackoverflow.com/help/mcve). Probably, missing context may be the actual reason for the observed behavior. – Scheff's Cat Oct 23 '17 at 05:27
  • @Scheff, I've added the code now. – a3y3 Oct 23 '17 at 05:30
  • If the declaration is `std::string words[n]` then the expression `words[j]` refers to the array subscript but not the overloaded operator `std::string::operator[]()`. To convince yourself, you could do `words[j][0]` which refers to the first element (character) in `words[j]`. – Scheff's Cat Oct 23 '17 at 05:31
  • `int n; cin << n; string words[n];` means you're working with a variable length array. This is supported in C11 but not in C++. If it works this is a proprietary extension of the compiler but I wouldn't count on it. In this case, `std::vector words;` is a proper C++ replacement. – Scheff's Cat Oct 23 '17 at 05:33
  • Are you sure that `string words[n]` does a proper construction of all elements? In opposition, `std::vector words(n);` would grant this. – Scheff's Cat Oct 23 '17 at 05:35
  • @Scheff "This is supported in C11" Well... sort of supported. It was made optional in C11. Check for `__STDC_NO_VLA__` before you try to use it. – user4581301 Oct 23 '17 at 05:42
  • Probably your error: `for(int i =1;i<=s.length();i++)` allows `i` to reach `s.length()` which is out of range for `s`. – user4581301 Oct 23 '17 at 05:48
  • @user4581301 But there is no `s[i]` access. – Gaurav Sehgal Oct 23 '17 at 05:50
  • @user4581301 This was alerting me too but: The only use of `i` is as 2nd arg. of `std::string::substr()`. The default is `npos` and this does limit the sub-string to `std::string::size()`. Thus, it shouldn't be _the_ cause of sigabort... – Scheff's Cat Oct 23 '17 at 05:51
  • @Scheff. You're right. I'm on crack. – user4581301 Oct 23 '17 at 05:54
  • [`using namespace std;` is a bad practice](https://stackoverflow.com/q/1452721/2176813), never use it. Additionally, `stdlib.h` and `stdio.h` are deprecated C-compatibility headers, always use `cstdlib` and `cstdio` respectively instead. – tambre Oct 23 '17 at 06:06
  • Please provide sample input and expected output. The question is not answerable otherwise. – tambre Oct 23 '17 at 06:07
  • @tambre, thanks, I'll keep that in mind. I've added the original question. – a3y3 Oct 23 '17 at 06:12
  • @Scheff, I'm not sure what you mean when you say "If the declaration is std::string words[n] then the expression words[j] refers to the array subscript but not the overloaded operator std::string::operator[]().". I tried deleting my code and using words[1].find(), and no error was thrown. – a3y3 Oct 23 '17 at 06:15
  • @aeye Never include code, input or output as pictures (accessibility problems) or on an external site (the site may go down), instead include it in the question and format it as a code block. Questions doing including any part of them on an external site are off-topic according to the rules. – tambre Oct 23 '17 at 06:28
  • I tried to answer your specific question "Isn't words[j] referring to a string element?" Yes, for declaration `string words[n];` `words[j]` is referring to the element j which is of type `string` (i.e. `operator[]` returns `string&`). – Scheff's Cat Oct 23 '17 at 06:32
  • I don't know how `words[j].find(new_word) != std::string::npos` blows up, but it could prevent a call to `s = s.erase(s.find(new_word), new_word.length())` [which blows up really easy](https://ideone.com/Op47nZ). Not so useful without an answer though, and I couldn't for the life of me figure out from the code what the goal was. – user4581301 Oct 23 '17 at 06:48
  • @user4581301 I'm really, really sorry for the horrible code. I'm still learning. The main goal of the program is, given an array that contains several words [I like xyz] and a string [ilike] is it possible to split the string such that the words are contained in the array. My approach was to iterate through the string, separate letters, first one, then 2, then 3, and keep checking if any match is found in the array. If it is found, it is deleted from the string. – a3y3 Oct 23 '17 at 13:57
  • I got that from the link you posted, which is why I commented on it. Things to try and prevent confusion: better problem description up front so readers know what the code is supposed to do. Better variable names to that readers know what the variable represents and can infer how it is supposed to be transformed and used. – user4581301 Oct 23 '17 at 15:33
  • Keywords: "backtracking" and "recursion". Say you have a function that removes word to remove from the front of a string to be parsed. If string to be parsed is now empty, return true. Otherwise, check to see if any of a list of strings is at the front of string to be parsed. If yes return the result of calling the function again with string to be parsed and the word that was found because it needs to be removed, if no, return false. – user4581301 Oct 23 '17 at 15:42
  • I'll keep that in mind. – a3y3 Oct 26 '17 at 07:47

1 Answers1

1

It's pretty simple why it's crashing when you change that line. In the version that crashes you check whether words[j].find(new_word) is equal to std::string::npos and then call s.erase(s.find(new_word), ...). Of course, s.find(new_word) could still be std::string::npos, and string::erase throws an exception when the first argument is out of range. In the version that you say does not crash, you correctly check whether s.find(new_word) is std::string::npos before passing it to s.erase. It actually has nothing to do with whether you're using a string in an array or not.

Be it known, though, the variable length array string words[n] is not standard c++, it's a GCC extension, and so you have to look to them for documentation as to whether all strings are initialized and and whatnot. The standard C++ way to make that array would be string *words = new string[n] followed by delete[] words to free the memory. If you do that, sizeof(words) will be the size of a string pointer, not the allocated size of the array, so you would have to use n instead of sizeof(words) / sizeof(words[0]). The current best-practice would be to use a shared_ptr<string> or a unique_ptr<string> and make_shared or make_unique depending on whether or not you will have multiple aliases to the array. If you use those, you do not have to call delete (or new for that matter); the memory will be freed correctly and automatically when the *_ptr objects go out of scope.

jcarpenter2
  • 5,312
  • 4
  • 22
  • 49