Use the tools you have
##Use the tools you have
TheThe first thing I notice is that you're doing some work you don't need to. You're using a std::map
to hold your keys and values, but you aren't utilizing the built-in method for finding an entry. (In other languages map
s are actually named dictionary
!) Your entire search_term()
method could look like this:
bool dictionary::search_term(const std::string& term){
std::map<std::string, std::string>::iterator it = entries.find(term);
return it != entries.end();
};
Likewise, the erase_entry()
method could just call the std::map::erase()
method.
Naming is hard
##Naming is hard
ForFor the most part your variable and method names are pretty good! There are a few that I'd change, though. The exists()
function should be something like file_exists()
so it's not confused with checking if an entry exists in the dictionary.
Also, ofs_entries()
and ifs_entries()
are odd names. I'd call them something like write_entries_to_file()
and read_entries_from_file()
.
##Use the tools you have
The first thing I notice is that you're doing some work you don't need to. You're using a std::map
to hold your keys and values, but you aren't utilizing the built-in method for finding an entry. (In other languages map
s are actually named dictionary
!) Your entire search_term()
method could look like this:
bool dictionary::search_term(const std::string& term){
std::map<std::string, std::string>::iterator it = entries.find(term);
return it != entries.end();
};
Likewise, the erase_entry()
method could just call the std::map::erase()
method.
##Naming is hard
For the most part your variable and method names are pretty good! There are a few that I'd change, though. The exists()
function should be something like file_exists()
so it's not confused with checking if an entry exists in the dictionary.
Also, ofs_entries()
and ifs_entries()
are odd names. I'd call them something like write_entries_to_file()
and read_entries_from_file()
.
Use the tools you have
The first thing I notice is that you're doing some work you don't need to. You're using a std::map
to hold your keys and values, but you aren't utilizing the built-in method for finding an entry. (In other languages map
s are actually named dictionary
!) Your entire search_term()
method could look like this:
bool dictionary::search_term(const std::string& term){
std::map<std::string, std::string>::iterator it = entries.find(term);
return it != entries.end();
};
Likewise, the erase_entry()
method could just call the std::map::erase()
method.
Naming is hard
For the most part your variable and method names are pretty good! There are a few that I'd change, though. The exists()
function should be something like file_exists()
so it's not confused with checking if an entry exists in the dictionary.
Also, ofs_entries()
and ifs_entries()
are odd names. I'd call them something like write_entries_to_file()
and read_entries_from_file()
.
##Use the tools you have
The first thing I notice is that you're doing some work you don't need to. You're using a std::map
to hold your keys and values, but you aren't utilizing the built-in method for finding an entry. (In other languages map
s are actually named dictionary
!) Your entire search_term()
method could look like this:
bool dictionary::search_term(const std::string& term){
std::map<std::string, std::string>::iterator it = entries.find(term);
return it != entries.end();
};
Likewise, the erase_entry()
method could just call the std::map::erase()
method.
##Naming is hard
For the most part your variable and method names are pretty good! There are a few that I'd change, though. The exists()
function should be something like file_exists()
so it's not confused with checking if an entry exists in the dictionary.
Also, ofs_entries()
and ifs_entries()
are odd names. I'd call them something like write_entries_to_file()
and read_entries_from_file()
.