Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser should use our new Queue class instead of FakePath #114

Open
awglyde opened this issue Feb 13, 2017 · 4 comments
Open

Parser should use our new Queue class instead of FakePath #114

awglyde opened this issue Feb 13, 2017 · 4 comments
Assignees
Labels

Comments

@awglyde
Copy link
Contributor

awglyde commented Feb 13, 2017

It looks like FakePath is wrapping the new Queue class. If we swap the FakePath class with the Queue class we can simplify some of the parser code.

@alexanderbenfox, you've worked on the Parser a lot. What do you think?

@alexanderbenfox
Copy link
Collaborator

the FakePath class only exists for debugging purposes. before we swapped out the queues, it served a purpose in being an intermediary between the path provided by the tester (as an array) and converted it to the path data type which the parser processes into a move list. if you can find a clean way to make the change, go ahead

@awglyde
Copy link
Contributor Author

awglyde commented Feb 13, 2017

I thought it only existed for debugging purposes as well however I found that FakePath is being used to create the relative path in PathParser::buildRelativePath which is called by the PathParser constructor. That relative path goes on to be used by a plethora of functions to determine the move list.

@ghost
Copy link

ghost commented Feb 14, 2017

@awglyde Oh yeah, FakePath does get used everywhere to build move_list. And it does look like pretty much a wrapper around Queue, but AFAIK it was meant to mimic a Path. Probably the reason a real Path wasn't used is that a Path has to be built from inside its own constructor, and that didn't work here?

Two options could be

  1. Replace FakePath with a plain Queue<Compass8, xx> like you said, or
  2. Attack Path data structure and path-finding algorithms are coupled #102, and then in theory that lets us use a plain Path instead of FakePath

Maybe it's even the case that in general, a Path should just be reduced to a Queue<Compass8, xx>. Or maybe a Queue doesn't even really fit the problem, maybe it should be basically an index-able array.

A bit of a tangle to sort out.

@ghost
Copy link

ghost commented Feb 14, 2017

Also. This line is ill-formed, it shouldn't be 50*sizeof(Compass8). Queue capacity is the max number of elements that can be stored, not the number of bytes.

@awglyde awglyde self-assigned this Feb 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants