There are a number of things that could be improved here.
Don't loop on eof()
It's almost always incorrect to loop on eof() while reading a file. The reason is that the eof indication is only set when an attempt is made to read something from the file when we're already at the end. In this case, it means we have already executed the line file2 >> num; and then called something like this x.push_back(stof(num));. The problem is that if we're at the end of the file and we attempt to read one more x value, we are now passing an empty string to stof. This is guaranteed to throw a std::invalid_argument exception. See this question for more details on why using eof is usually wrong.
Use better naming
It's not clear from the question whether ModelLoader is a class or a namespace. If it's a namespace, then it appears that vertexArray is a global variable, which is a problem itself. If it's a class, and vertexArray is a member variable, then I'd suggest reconsidering both names. If the vertexArray is part of the model, as it appears, then I'd suggest that the class should be Model and the member function should be load. Note that I use the common convention of having class names in CamelCase and members in lowercase. A consistently applied naming convention helps readers understand the code.
Use const where practical
The LoadVertex() function does not need to modify the passed string and doesn't need to make a copy, but here's how it's declared:
void LoadVertex(std::string fileLocation)
Instead, we can both avoid making a copy and signal that the passed string is not modified by doing this instead:
void LoadVertex(const std::string &fileLocation)
Return something useful from a function
It's a frequent newbie mistake to make all functions void. Better practice is to return something useful from the function. In this case the function includes this mysterious comment:
//clears vector so it doesnt break when loading multiple models
vertexArray.clear();
This strongly suggests to me that the vertexArray probably shouldn't be a part of the class at all. Instead, it probably makes more sense to create the vertexArray and then return it from the function as:
std::vector<PhysicsEngine::PxVec3> LoadVertex(const std::string &fileLocation);
Initialize variables on declaration
The best practice is to initialize varibles as soon as they're created. In C++, this most often means initializing them in the same line as the declaration. So instead of this:
std::ifstream file2;
file2.open(fileLocation);
write this:
std::ifstream file2{fileLocation};
Note, too, that I use the C++11 uniform initializer syntax (with the {}) to make it clear to both the compiler and the reader that this is an initialization and not a function call. See Stroustrup's description for more details on that.
Avoid adding unnecessary objects
The three vectors for the three coordinates are not really needed at all. Instead, one could simply read three floats and create the PxVec3 objects directly. Here is one way to do that:
namespace ModelLoader {
std::vector<PhysicsEngine::PxVec3> LoadVertex(const std::string &fileLocation)
{
std::vector<PhysicsEngine::PxVec3> vertexArray;
std::ifstream file2{fileLocation};
float x, y, z;
while (file2 >> x >> y >> z) {
vertexArray.emplace_back(x, y, z);
}
return vertexArray;
}
}
However, I'd actually suggest the following approach instead.
Create an object reader
Instead of having an external class explicitly construct a PhysicsEngine::PxVec3 class instance, it makes more sense to me to have a custom object reader defined within that class and then call that. Here's what I mean, using a minimal PxVec3 class:
namespace PhysicsEngine {
class PxVec3 {
public:
PxVec3(float x=0, float y=0, float z=0) :
x{x}, y{y}, z{z}
{}
// a custom object reader
friend std::istream &operator>>(std::istream &in, PxVec3 &vec) {
return in >> vec.x >> vec.y >> vec.z;
}
private:
float x, y, z;
};
}
Now we can rewrite the loader slightly differently:
namespace ModelLoader {
std::vector<PhysicsEngine::PxVec3> LoadVertex(const std::string &fileLocation)
{
std::vector<PhysicsEngine::PxVec3> vertexArray;
std::ifstream file2{fileLocation};
PhysicsEngine::PxVec3 point;
while (file2 >> point) {
vertexArray.emplace_back(point);
}
return vertexArray;
}
}
Note that now, if we want to use a double instead of a float within the PxVec3 class, no change is required to this code. This is the advantage of information hiding, that is, keeping class implementation details private to the class. However, there's an even simpler way to do this, which uses the standard library:
std::ifstream in{filename};
std::vector<PhysicsEngine::PxVec3> model{
std::istream_iterator<PhysicsEngine::PxVec3>{in},
std::istream_iterator<PhysicsEngine::PxVec3>{}
};
This declares model as a vector and then uses std::istream_iterator and the previously defined object reader to very elegantly initialize the vector.
