Simple user registry code in C++

Question

I'm learning C++, and would like more experienced folks to review my code, and point out what could be improved/changed.

Here's the problem that should be solved (taken from Kattis Securedoors)

You need to write a simple software auditor for tracking employees entering and exiting your building using their access cards (each card uniquely identifies the employee holding the card). Your software needs to look at the access log and determine if there are any anomalies. There are two types of anomalies your software should detect:

• a user entering the building when, according to the log, they are already supposed to be in the building
• a user exiting the building when, according to the log, they are not supposed to be in the building

When your software sees someone enter a building (even if it’s an anomaly), that person is assumed to be inside the building from that point on, until he exits. Similarly, if your software sees someone exit the building (even if it’s an anomaly), that person is assumed to not be in the building from that point on, until he enters again. At the beginning of the log, everyone is assumed to be outside the building.

Input

Input Input starts with a number 1≤n≤10001≤n≤1000 indicating the length of the log. This is followed by nn lines, each line describing either an entry or exit by an employee. Each description is of the form ‘entry name’ or ‘exit name’, where name is a string of up to 20 uppercase and/or lowercase characters (a-z).

Output

For each person’s entry or exit, print the name of the person, followed by entered or exited. If the action is anomalous, print (ANOMALY) afterward.

Sample Input 1

8
entry Abbey
entry Abbey
exit Abbey
entry Tyrone
exit Mason
entry Demetra
exit Latonya
entry Idella

Sample Output 1

Abbey entered
Abbey entered (ANOMALY)
Abbey exited
Tyrone entered
Mason exited (ANOMALY)
Demetra entered
Latonya exited (ANOMALY)
Idella entered

Here's my code (working sample)

#include <iostream>
#include <unordered_map>
#include <string>
class UserRegistry {
public:
 void UserEntered(const std::string& user) {
 RegisterUserAction(user, "entered");
 }
 void UserExited(const std::string& user) {
 RegisterUserAction(user, "exited");
 }
private:
 void LogAction(const std::string& user, const std::string& action, bool is_anomaly = false) {
 std::cout << user << " " << action << (is_anomaly ? " (ANOMALY)" : "") << std::endl;
 }
 void RegisterUserAction(const std::string& user, const std::string& action) {
 bool anomaly = false;
 if (action == "entered" && registry[user] % 2 == 1) {
 anomaly = true;
 }
 if (action == "exited" && registry[user] % 2 == 0) {
 anomaly = true;
 }
 if (!anomaly)
 registry[user]++;
 LogAction(user, action, anomaly);
 }
private:
 std::unordered_map<std::string, int> registry;
};
int main() {
 UserRegistry registry;
 int log_length;
 std::cin >> log_length;
 for (int i = 0; i < log_length; ++i) {
 std::string user, action;
 std::cin >> action >> user;
 if (action == "entry")
 registry.UserEntered(user);
 else if (action == "exit")
 registry.UserExited(user);
 }
 return 0;
}

• πάντα ῥεῖ: Please do those edits yourself next time!
• akmal: @πάνταῥεῖ, I was going to add it later today, thanks anyway :)!
• πάντα ῥεῖ: Don't procrastinate anything when asking questions here. Provide them as perfect you can in 1st place. As you can see, that would only waste every other ones precious time.

Answer 1

std::unordered_map<std::string, int> is an overkill. Since you only need to track of a presence - which is naturally boolean - std::set<std::string> is enough. As a perk benefit, notice that std::set::insert indicates whether the insertion was successful or not (this is an anomaly), and std::set::erase return a number of elements erased (0 means the key was not there, another anomaly):

 std::set<std::string> registry;
 bool UserEntered(const std::string& user) {
 return !registry.insert().second;
 }
 bool userExited(const std::string& user) {
 return registry.erase(user) == 0;
 }

Now both methods return the anomaly:

 bool anomaly;
 if (action == "entered") {
 anomaly = userEntered(user);
 } else if(action == "exited") {
 anomaly = userExited(user);
 } else {
 // handle error here
 return;
 }
 LogAction(....);

Answer 2

I don't really get what that code is finally purposed for. It looks like the UserRegistry class finally counts how often a user entered, correct me if I'm wrong about that.
That said, why is exiting always considered an anomaly??

---

Another minor point:

if (!anomaly)
 registry[user]++;
if (action == "entry")
 registry.UserEntered(user);
else if (action == "exit")
 registry.UserExited(user);

The above statements miss the scope blocks for the single line statements. That works, yes, but it's bad style IMO and error prone for mainenance. Besides that you should always use styles consistently, as here

if (action == "exited" && registry[user] % 2 == 0) {
 anomaly = true;
}

you don't keep with it.

• akmal: sorry, here you can find description of the problem which I'm trying to solve. open.kattis.com/problems/securedoors
• πάντα ῥεῖ: @akmal Put that background in your question please (and please not just the link).