Interesting: The object reads the data, but doesn't return anywhere nor does it expose it. Is it by design? I guess not ;)
Peculiar function: read_list()
returns an int, but the result is discarded on the call site anyway. No exceptions, just the error printed out. Since the code doesn't perform any error handling anyway, the vectors construction could look much better:
Read_Data::Read_Data():
int_list(std::istream_iterator<int>(std::ifstream("ints.txt")), {}),
string_list(std::istream_iterator<std::string>(std::ifstream("strings.txt")), {})
{}
But the std::istream_iterator
has constructor that only takes non-const lvalue reference to the stream (not that const version would be useful anyway), which leads me to the next points:
The object is an algorithm, but pretends to be an object: I think that the class should be refactored to be a function which accepts two vectors (with int and string list) and a parsed json.
Usually I make bare bones interface which has greatest power and doesn't take usability into account. Then I make some higher level functions which just call lower level functions, which are meant to simplify things and make them easier to use. With that in mind:
auto separate_info(const nlohmann::json& content,
const std::vector<std::string>& int_list,
const std::vector<std::string>& string_list)
{
std::map<std::string, int> int_list;
std::map<std::string, std::string> string_list;
/*populate them*/
return std::pair{int_list, string_list};
}
auto read_info() //some defaulted parameters if needed
{
std::ifstream int_parameter_list{"..."};
std::ifstream string_parameter_list{"..."};
template <typename T>
using isiter = std::istream_iterator<T>;
std::vector<std::string> int_attributes(isiter<int>{int_parameter_list}, {});
std::vector<std::string> text_attributes(isiter<std::string>{string_parameter_list}, {});
//parse the json
return separate_info(j, int_attributes, text_attributes);
}
Then on the call site:
auto [int_list, string_list] = separate_info(...);
The names could've been better: naming is hard. May be there is something in common between them? What do they refer to? I guess it is something related to a person. The maps could be packed into person_info
if my guess was right. It won't be that significant, but maybe pair won't be a good choice if the read data is used in many places.
struct person_info
{
std::map<...> int_info;
std::map<...> text_info;
}
And the last, not so important point:
Premature optimization, or ignorance: I don't mean to be offending, but returning vector from a function is probably very cheap. One thing is NRVO, which will probably kick in since it is gonna be created on the first line and returned from one point. If NRVO will fail, move constructor will kick in, so it is gonna be something like copying/swapping of 3 pointers. Although standard doesn't require this, most compilers do it. If you're on mainstream, e.g. gcc/clang/vc++, then returning vector shouldn't be a problem.