Parameter shadowing a local variable:
The first issue with your code that prevented in from compiling with Clang is the
inFile parameter of getInput() that shadows the ifstream instance of the same name.
Your compiler seems to be more forgiving with that problem. If you are using GCC, I recommend using the -Wshadow warning flag to enable warnings of this kind of variable name shadowing/redeclaration. This category of bugs can be particularly hard to track in a scenario where both variables are of the same type.
Reserved suffix:
The _t suffix is reserved by the POSIX standard (read this), so it is best if avoided.
Consider renaming direction_t and priority_t to Direction and Priority.
Const correctness:
The filename parameter of the function is a read-only string, so it should at least
be a const char *. But since this is C++ code, prefer a standard string and pass it by const reference:
void getInput(const std::string & filename)
Error handling:
You did right by using std::cerr to report errors, however, exit()ing after the error is logged
seems like a very crude way of handling it. Consider throwing an exception or even
making the function return a boolean false.
Using namespace:
You are probably using namespace std somewhere, judging by the lack of the std:: prefix in the library calls. This is best if avoided as it defeats the purpose of a namespace, which is allowing equal names to coexist without clashes. Read this thread for a more in-depth discussion.
Data driven solution:
There is a more data driven and flexible way of handling the conversion between the chars
read from file to the Direction/Priority enums than an if chain or a switch statement.
You could instead use a map keyed by a char. This way, expanding and adding more chars
is simpler and involves less code change. You could even externalize the map to a file
and load it dynamically at runtime if you wanted.
Following is the refactored code with the changes above mentioned applied to it plus
the use of a std::map as a lookup table. The map associates a char (the key)
with a pair of [Direction, Priority]. Making the conversion form the file data a lot more concise.
#include <iostream>
#include <fstream>
#include <string>
#include <map>
bool getInput(const std::string & filename)
{
enum Direction { NORTH, SOUTH };
enum Priority { HIGH, LOW };
const std::map<char, std::pair<Direction, Priority>> lookupTable = {
{ 'n', { NORTH, LOW } },
{ 'N', { NORTH, HIGH } },
{ 's', { SOUTH, LOW } },
{ 'S', { SOUTH, HIGH } }
};
Priority priority;
Direction direction;
std::string line;
std::ifstream inFile(filename);
if (!inFile.good())
{
std::cerr << "can't open input file";
return false;
}
while (getline(inFile, line))
{
const char key = line.at(0); // first letter signifies direction and priority
const auto entry = lookupTable.find(key);
if (entry != lookupTable.end())
{
direction = entry->second.first;
priority = entry->second.second;
}
else
{
std::cerr << "invalid direction given: " << key << std::endl;
}
size_t commaIndex = line.find(",", 2); // know first character is letter, second is |
std::string firstNumber = line.substr(2, commaIndex - 2);
std::string secondNumber = line.substr(commaIndex + 1, line.length());
std::cout << "firstNumber: " << firstNumber << " secondNumber " << secondNumber << std::endl;
}
return true;
}
So it turn out that the OP's compiler is missing a few C++11 features that prevent the code provided above from compiling. It can be fairly easily adapted to support older compilers though, if we are willing to add a few not so elegant workarounds. Here is one way to adapt it using an aux function that keeps a static std::map:
enum Direction { NORTH, SOUTH, DIR_INVALID };
enum Priority { HIGH, LOW, PRI_INVALID };
std::pair<Direction, Priority> lookup(char key)
{
typedef std::map< char, std::pair<Direction, Priority> > TableType;
static bool isTableInitialized = false;
static TableType lookupTable;
// One time initialization
if (!isTableInitialized)
{
lookupTable.insert(std::make_pair( 'n', std::make_pair(NORTH, LOW) ));
lookupTable.insert(std::make_pair( 'N', std::make_pair(NORTH, HIGH) ));
lookupTable.insert(std::make_pair( 's', std::make_pair(SOUTH, LOW) ));
lookupTable.insert(std::make_pair( 'S', std::make_pair(SOUTH, HIGH) ));
isTableInitialized = true;
}
TableType::const_iterator entry = lookupTable.find(key);
if (entry != lookupTable.end())
{
return (*entry).second;
}
// Not found / invalid key
return std::make_pair(DIR_INVALID, PRI_INVALID);
}
bool getInput(const std::string & filename)
{
Priority priority;
Direction direction;
std::string line;
std::ifstream inFile(filename.c_str());
if (!inFile.good())
{
std::cerr << "can't open input file";
return false;
}
while (getline(inFile, line))
{
// first letter signifies direction and priority
const char key = line.at(0);
// Lookup the conversion table:
const std::pair<Direction, Priority> entry = lookup(key);
if ((entry.first != DIR_INVALID) && (entry.second != PRI_INVALID))
{
direction = entry.first;
priority = entry.second;
}
else
{
std::cerr << "invalid direction given: " << key << std::endl;
}
size_t commaIndex = line.find(",", 2); // know first character is letter, second is |
std::string firstNumber = line.substr(2, commaIndex - 2);
std::string secondNumber = line.substr(commaIndex + 1, line.length());
std::cout << "firstNumber: " << firstNumber << " secondNumber " << secondNumber << std::endl;
}
return true;
}
It has its drawback, but should work just the same. It is also quite a bit more code to get it done. One important thing to keep in mind is that initialization order of the lookup table is not thread safe, so don't try to use this in a concurrent program.
