search

udacity / CarND-MPC-Project

CarND Term 2 Model Predictive Control (MPC) Project
MIT License
280 stars 1.2k forks source link

fix missing namespace so that code compiles #39

Closed shunjilin closed 5 years ago

mvirgo commented 5 years ago

Can you clarify where you were having issues with this from the starter code? Would also help to know your OS and compiler. It appears to compile fine without using namespace std, and string is already included because json.hpp is the first include within main.cpp.

shunjilin commented 5 years ago

I am using Ubuntu 18.04, gcc compiler and I installed the external dependencies locally, according to the instructions.

Perhaps something has changed in Ipopt and CppAD.

I guess as Json.hpp is not an external dependency, not explicitly including string might be fine.

shunjilin commented 5 years ago

Unrelated, but the virtual destructor and constructor for MPC seems to be redundant as MPC is not used in a polymorphic way

mvirgo commented 5 years ago

Wanted to leave an update here that I'm going through to refactor all of our C++ code in lessons and projects, so I'll come back to this once I get up to MPC (I'm through the Sensor Fusion content and will be starting Localization soon).

As part of that, I think you are probably right to have string included, but we won't use using namespace std, as that actually scopes in way too much. See a related article here.

We do currently do that in a lot of lessons, but I'm trying to re-do a lot of quizzes to only use what we need, e.g. using std::cout, so that it shows best practices.

shunjilin commented 5 years ago

Agree, that would be better. Is there a way that someone can help out with testing the new implementations? I would be interested in revisiting the exercises in this term.

mvirgo commented 5 years ago

I'm going to go ahead and merge this for now just so students aren't having issues in the interim, since it's taking awhile to get to this one.

For the most part the implementations are just changing from a style-side, so not much to test, although I will certainly let you know if that changes!