Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

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 maps 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 maps 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 maps 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().

Source Link
user1118321
  • 11.9k
  • 1
  • 20
  • 46

##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 maps 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().

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /