Could Use Guidance from the Experts

[Mogo Mojo]

I'm pretty new to C++, using this group and a Safari InformIT membership asmy primary learning resources. I have a difficult time learning without aprogramming problem that "feels" real (don't really get motivated by Dogs.bark(), I knew that before I started learning a programming language).

That being said, I wrote a small program that I can actually use in my day job. It works and I learned a lot struggling through it, but I was hoping you guys could point out where I could improve it, make it more efficient, and possibly catch bad habits I may be forming. Especially looking for a better solution than my nested for loops in getActiveList().

I do testing at companies and need to pull random names from a list of employees. I also have a list of currently laid off employee IDs. The idea isto pull those lists, compare them and end up with a random list of n currently active employees. I can do this with an Access query, but my laptop doesn't have Access. This allows me to pull a random name on site from my laptop if one of the selected employees is absent.


// The private methods of EmpRecord

vector<string> EmpRecord::getInactiveList(){
vector<string> inactiveEmps; // contain employee IDs only
ifstream is; // laid off employees
string fieldValue;

is.open("Inactive.txt");
while(getline(is, fieldValue))
inactiveEmps.push_back(fieldValue);
is.close();

return inactiveEmps;
}

vector<string> EmpRecord::getEmpList(){
ifstream is;
string fieldValue;
vector<string> emps; // .csv list of employees
// ID,"Name",DoB
is.open("EmployeeList.csv");
while(getline(is, fieldValue))
emps.push_back(fieldValue);

is.close();

return emps;
}

vector<string> EmpRecord::getActiveList(vector<string> a, vector<string> na){ // Uses the returns of getEmpList() and getInactiveList()
vector<string> activeEmps; // currently active employees..
for(auto si : a){ // a better way to do this?
bool isActive = true;
auto nStart = si.find('"', 0);
for(auto sii : na){
if(si.substr(0, nStart - 1) == sii){
isActive = false;
break;
}
}
if(isActive)
activeEmps.push_back(si);
}

return activeEmps;
}

// The public methods of EmpRecord

vector<string> EmpRecord::Random(size_t n){ // main entry point into class

vector<string> allEmps = EmpRecord::getEmpList();
vector<string> inactiveEmps = EmpRecord::getInactiveList();
vector<string> activeEmps = EmpRecord::getActiveList(allEmps, inactiveEmps);

n <= activeEmps.size() ? n : n = activeEmps.size(); // upper bound check

srand(time(NULL));

set<int> vi; // Using a set for unique values.
while(vi.size() < n) // Keep iterating until vi has n employees.
vi.insert(rand() % activeEmps.size());

vector<string> randList;
for(auto i : vi)
randList.push_back(activeEmps);

return randList;
}

void EmpRecord:rintRandList(vector<string> s){

for(auto i : s){
cout << i << endl;
}
}

Thanks for any insights and constructive criticism you can share.

 

[red floyd]

I'm pretty new to C++, using this group and a Safari InformIT membership as my primary learning resources. I have a difficult time learning without a programming problem that "feels" real (don't really get motivated by Dogs.bark(), I knew that before I started learning a programming language).

That being said, I wrote a small program that I can actually use in my day job. It works and I learned a lot struggling through it, but I was hoping you guys could point out where I could improve it, make it more efficient, and possibly catch bad habits I may be forming. Especially looking for a better solution than my nested for loops in getActiveList().

I do testing at companies and need to pull random names from a list of employees. I also have a list of currently laid off employee IDs. The idea is to pull those lists, compare them and end up with a random list of n currently active employees. I can do this with an Access query, but my laptop doesn't have Access. This allows me to pull a random name on site from my laptop if one of the selected employees is absent.

Sort the Active and NonActive list vectors. Then run through them together.

Something like (PSEUDOCODE WILL NOT COMPILE):

sort(a.begin(), a.end());
sort(na.begin(), na.end());

vector::iterator a_iter = a.begin();
vector::iterator na_iter = na.begin();
while (a_iter != a.end() && na_iter != na_end())
{
if (a_iter->name == na_iter->name)
{
a_iter->active = false;
++a_iter;
++na_iter;
}
else if (a_iter->name < na_iter->name)
a_iter->active = true;
++a_iter;
}
else // a > na
{
++na_iter;
}
}

 

[Mogo Mojo]

Sort the Active and NonActive list vectors. Then run through them together.



Something like (PSEUDOCODE WILL NOT COMPILE):



sort(a.begin(), a.end());

sort(na.begin(), na.end());



vector::iterator a_iter = a.begin();

vector::iterator na_iter = na.begin();

while (a_iter != a.end() && na_iter != na_end())

{

if (a_iter->name == na_iter->name)

{

a_iter->active = false;

++a_iter;

++na_iter;

}

else if (a_iter->name < na_iter->name)

a_iter->active = true;

++a_iter;

}

else // a > na

{

++na_iter;

}

}

Not sure if this is possible, but can I dereference the a_iter to get to the value so I can use the substr(0, nStart - 1) on it to get only the ID from that vector? The a vector contains a full comma separated list of ID,"Name",DoB and the na vector contains only ID lines. I don't know what it would look like, but maybe (*a_iter.begin()).substr(0, nStart - 1) maybe? If not, I could always build the inactive list up to match the format of the full employee csv. My knowledge of pointers and references is still pretty limited.