Dictionary file manager

Question

The dictionary class loads entries from a file and then can perform operations on them. The dictionary class then stores the entries back into the file.

I would like to know the best way to check if the arguments to my dictionary member functions are valid terms/definitions. I have thought of two possible solutions, but I am open to any other solutions. I could either check the arguments passed in to the functions or make a term and definition class and and have the class check instead.

The reason why I want to distinguish strings from terms/definitions is so that I can read them from a file without having to encounter cases where definitions do not end in periods or other cases caused by human error.

dictionary.cpp

#include "dictionary.h"
//*** @TODO: What constitutes a term/definition ***
bool dictionary::search_term(const std::string& term){
 for(auto& it: entries){
 if(it.first != term);
 else return true;
 }return false;
};
bool dictionary::erase_entry(const std::string& term){
 if(search_term(term)){
 entries.erase(term);
 return true;
 }else return false;
};
bool dictionary::define_term(const std::string& term, const std::string& definition){
 if(search_term(term)){
 entries[term] = definition;
 return true;
 }else return false;
};
bool dictionary::write_entry(const std::string& term, const std::string& definition){
if(!search_term(term)){
 entries[term] = definition;
 return true;
 }else return false;
};
inline bool exists (const std::string& name) {
 struct stat buffer; 
 return (stat (name.c_str(), &buffer) == 0); 
}
bool dictionary::ofs_entries(const std::string& path){
 std::string file = (path + title);
 std::ofstream ofs(file.c_str());
 if(!ofs) return false;
 for(auto& it: entries){
 ofs << it.first << ": " << it.second << '\n';
 }ofs.close();
};
bool dictionary::ifs_entries(const std::string& path){
 std::string file = (path + title);
 if(!exists(file)) return false;
 std::ifstream ifs(file.c_str());
 if(!ifs) return false;
 std::string entry;
 while(true){
 //read entries
 if(!ifs.eof()) break;
 }return true;
};

dictionary.h

#ifndef DICTIONARY_H
#define DICTIONARY_H
#include <algorithm>
#include <iterator>
#include <fstream>
#include <string>
#include <map>
class dictionary{
 public:
 dictionary(const std::string& title, const std::string& definition = "")
 : entries{{title, definition}}, title(title){;};
 bool write_entry(const std::string& term, const std::string& definition = "");
 bool define_term(const std::string& term, const std::string& definition);
 bool erase_entry(const std::string& term);
 bool search_term(const std::string& term); 
 bool ofs_entries(const std::string& path);
 bool ifs_entries(const std::string& path);
 private:
 std::map<std::string, std::string> entries;
 std::string title;
};
#endif//DICTIONARY_H

• Morwenn: Could you please provide the declaration of dictionary? We won't be able to help you with only one half of the information we need :/
• dylan: The dictionary class loads entries from a file and then can perform operations on them. The dictionary class then stores the entries back into the file. I changed the title to fit the description.

Answer 1

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

• dylan: Should I make the search function inline?
• user1118321: Marking things inline is optional. If you do, the compiler may still ignore it, and if you don't, the compiler may still decide to inline it anyway. To know whether it helps or not, you need to profile it in your actual code with data that's similar to the actual data your users will be using.