0

There are total of 7 spaces, 3 leftmost spaces contain a frog each of one family while the 3 rightmost spaces contain a frog each of second family. The 3 frogs on left need to be transferred to 3 rightmost positions while 3 frogs on right need to be transferred to 3 leftmost positions, but the condition is that each and every one of the frog can jump either to next position in left or right of it (if empty) or to two places to left or right (if empty). Find a suitable set of instructions to get to final position

Initial State(Input): FROG1 FROG1 FROG1 ___ FROG2 FROG2 FROG2

Final State(Output): FROG2 FROG2 FROG2 ___ FROG1 FROG1 FROG1 (where FROG1 denotes frog of first family and FROG2 denotes frog of second family)

I don't know where is it going wrong but after running the code, it shows a warning in swap function that s[index] may be uninitialized. The output is blank whereas expected output should be the steps how the frogs crossed.

My Idea to solve:

The root of this tree is initial state, lets denote the state at one time by "x" to denote frog of 1st family, "o" to denote frog of other family, "-" to denote space. So, the initial state becomes: ooo_xxx and final state is xxx_ooo The root node contains xxx_ooo, and information of its 4 children and its parent. To generate children of a node, just move the space ("-") to left, right, left to left and right to right of its position in parent node, if the state allows, otherwise the child will be null. Keep building the tree until the goal state is found and once the goal state is found print the path in reverse order from that node to root using the pointer of its parent

Approach:

#include <bits/stdc++.h>
using namespace std;

const string finalState = "xxx_ooo";
const string initialState = "ooo_xxx";

struct tree {   
    string s;
    struct tree *one, *two, *three, *four;
    struct tree *parent;
};

struct tree *newNode(string str) {
    struct tree *node;
    node = (struct tree*) malloc(sizeof(struct tree));
    node->s = str;
    node->one = nullptr;
    node->two = nullptr;
    node->three = nullptr;
    node->four = nullptr;
    return node;
}

void display(struct tree *node) {
    struct tree * par = node->parent;
    while(node->parent != nullptr) {
        cout << node->s << endl;
        node = par;
        par = node->parent;
    }
}
string nextState(string s, int choice) {
    int index;
    for(int i = 0; i < static_cast<int>(s.size()); i++) {
        if(s[i] == '_') {
            index = i;
        }
    }    
    if(index + choice >= 0 && index + choice < 7)
        swap(s[index], s[index + choice]);
    return s;
}

void insertUpdated(struct tree *curr, string str, int num) {
    switch(num) {
        case 1: curr->one = newNode(str);
            curr->one->parent = curr;
            break;
        case 2: curr->two = newNode(str);
            curr->two->parent = curr;
            break;
        case 3: curr->three = newNode(str);
            curr->three->parent = curr;
            break;
        case 4: curr->four = newNode(str);
            curr->four->parent = curr;
            break;
    }
}

void generateChild(struct tree *root) {
    queue<struct tree*> q;
    struct tree* ans;
    q.push(root);
    int cnt = 0;
    while(true) {
        struct tree *temp = q.front();
        q.pop();
        cnt++;
        if(temp->s == finalState){
            ans = temp;
            break;
        }
        string s1, s2, s3, s4, curr = temp->s;
        s1 = nextState(curr,-2);
        s2 = nextState(curr,-1);
        s3 = nextState(curr,1);
        s4 = nextState(curr,2);
        insertUpdated(temp, s1, 1);
        insertUpdated(temp, s2, 2);
        insertUpdated(temp, s3, 3);
        insertUpdated(temp, s4, 4);
        q.push(temp->one);
        q.push(temp->two);
        q.push(temp->three);
        q.push(temp->four);
    }   
    display(ans);
}

int main() {
    struct tree *root = newNode(initialState);
    generateChild(root);    
    return 0;
}
  • `node = (struct tree*) malloc(sizeof(struct tree));` is very wrong in C++. Please don't use competition sites as a learning resource, because all they teach is how to be bad at programming. Take classes, read [some good books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282), learn C++ *properly* (as well as common data structure and algorithms) before using such sites. – Some programmer dude Jul 26 '20 at 12:51
  • I believe that that part of code is correct and competitive programming is very good, the problem with my code is that I am struck in an infinite loop in the while statement, that is, the final state is unreachable. I am unable to solve that part. – kb1024bytes Jul 26 '20 at 14:17
  • That allocacion doesn't construct the `string` object `s`. Leading to undefined behavior when you assign to `s`. As a general rule, never use `malloc` in C++. – Some programmer dude Jul 26 '20 at 14:22
  • And competitive programming is good, *if you' already know how to code correctly*. It's not a learning or teaching resource, as I already mentioned. Besides the `malloc` issue, you have many other really bad habits show-cased in your code, bad habits that will fail any employment if you do these at interviews. – Some programmer dude Jul 26 '20 at 14:25
  • Sir, I am just trying to see whats going wrong here, but could you elaborate on "bad code" part in my code? – kb1024bytes Jul 26 '20 at 14:32
  • Besides the `malloc`? For example [`#include `](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h), and [`using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). All those "short cut" macros, the two first statements in the `main` function, lack of any kind of comments, using `int` instead of `size_t` for sizes/indexes, little to no error checking or input validation, use of distinct variables instead of e.g. arrays... – Some programmer dude Jul 26 '20 at 15:57
  • ... [continued] using the obsolete `NULL` symbolic constant, no `default` case in your `switch`, copy-pasted statements instead of functions gathering common code. Your indentation is nice, I'll give you that, and that makes it seem like you're not a total beginner which makes many of the other things all the more troubling. – Some programmer dude Jul 26 '20 at 15:57
  • @Someprogrammerdude Some people say malloc is not as bad as the rumors say but you say it is a terrible idea care to explain?? – Yunfei Chen Jul 26 '20 at 21:12
  • 1
    @Someprogrammerdude I would be more worried about #define fi first #define se second #define pb(x) push_back(x) #define sz(x) static_cast ((x).size()) Not good coding style....... Unreadable..... – Yunfei Chen Jul 26 '20 at 21:13
  • @Someprogrammerdude I was told that default was optional in college?? Also what is the difference between NULL and nullptr?? – Yunfei Chen Jul 26 '20 at 21:15
  • 1
    @YunfeiChen `malloc` only allocates memory, it doesn't invoke constructors. For [POD types](https://en.cppreference.com/w/cpp/named_req/PODType) (basically plain C-compatible structures) it's okay, but for anything else it's catastrophic. Consider the structure in the question, which includes a `std::string`. This string object will not be constructed, and the assignment to it will lead to *undefined behavior*. As a general rule, always use `new` for dynamic allocation in C++. – Some programmer dude Jul 27 '20 at 08:35
  • @YunfeiChen Without a `default` case in a `switch`, any case not handled will be undetected. If the possible values are validated and range-checked somewhere else then it might be okay, but otherwise there could be an error condition that is not handled or even detected. – Some programmer dude Jul 27 '20 at 08:37
  • 1
    @YunfeiChen Lastly about `NULL` and `0` and `nullptr`... `NULL` is a backward-compatibility macro left over from the "C with classes" early days of C++. It's implementation defined what the macro expands to (IIRC), but usually expands to the `int` constant `0`. In many cases where a pointer is expected the compiler will treat `0` as a null pointer. But think of a case with two overloaded functions, one taking an `int` value as argument, and the other a pointer. Is `0` a null pointer, or the zero integer value? With `nullptr` there's no ambiguity. – Some programmer dude Jul 27 '20 at 08:40
  • There are so many things that are wrong with your coding style, I could write a 300 page book about it. But your algorithm is totally wrong. You are generating the same state again and again and again and again and again and again and again, eventually exhausting all memory. Your search space *is not a tree*, trying to explore it with a tree data structure is not a good idea. – n. m. could be an AI Jul 27 '20 at 10:04
  • Ok so I used my template that I generally use on codeforces which includes those macros, I will keep in mind that I won't use them for other codes. And yes you are correct that my functions are bad, I didn't gave it much thought while writing that code. – kb1024bytes Jul 28 '20 at 09:34
  • And yes, I am generating same states again and again and again and again. What should I change in the code to correct that? – kb1024bytes Jul 28 '20 at 09:35
  • Also, I havent used default in switch because the function call has only const values so, there wont be any unwanted value in switch – kb1024bytes Jul 28 '20 at 09:43
  • I changed some part of code, now can you guys tell me what to do in order to not let repeatitive states generate – kb1024bytes Jul 28 '20 at 09:51
  • Try reading something about graph algorithms. Does the name Dijkstra ring a bell? – n. m. could be an AI Jul 28 '20 at 16:26
  • so you are suggesting to use some kind of open and closed set of nodes to restrict the generation of same nodes again and again. But then, there should be all possible nodes for which I will have to build a graph which seems to be kinda sketchy. Even so, I will give it a try – kb1024bytes Jul 29 '20 at 04:10
  • So guys I figured it out, the initial comparisons were taking something like 4^16 (1e9 approx) states and maybe the program ran out of memory but now I have used a set to not allow repetitive states to get stored, this reduced the number of states from 1e9 to 140. I think it's a huge optimisation in terms of memory, also I explored only new states so in terms of time complexity it's less than previous approach but I can't prove it's time complexity. Just wanted all of you to know it works now as few of you said it's a bad code and won't work. – kb1024bytes Aug 21 '20 at 11:04

0 Answers0