1

As an exercise, I recently implemented a simple settings reader class which reads setting values from an INI file into an std::map<std::string, std::string>. The reader method looks like the following:

std::map<std::string, std::string> readSettings() {
    std::map<std::string, std::string> settings;

    for(std::string rawSetting; std::getline(settingsFile, rawSetting); )
        settings.insert(parseRawSettingIntoPair(rawSetting));

    return settings;
}

I feel a bit redundant specifying the type of renturn value twice:

  • firstly in the method declaration,
  • secondly when declaring the local variable, settings.

I know that in the compiler can deduce the return type if I use the auto specifier in the declaration of readSettings(), but this feature will force me to keep its definition in the header file too. Is there any better approach to reduce such repetitions?

7
  • I don't know how/if you do this in C++, but writing this in a functional style would remove the redundancy and also prevent potential bugs where you modify the collection of settings. I can give an example but it wouldn't be in c++! Nov 29, 2017 at 17:01
  • 1
    Many approaches, not necessarily better. In your case it might be better having the map in a instance member instead of returning it. It makes sense and reduces the number of type specifications in your method to 0. BTW there is no redundancy in your code, both type specifications convey different bits of information that are necessary in different contexts. Nov 29, 2017 at 17:50
  • @Goyo, thank you for your suggestions. The responsibility of my SettingsReader class is to read settings from file into a container, but eventually it could be the container itself which hides how become filled from file.
    – Akira
    Nov 30, 2017 at 8:51
  • I'm new in this community, so I'm curious about the reasons of downvote to learn what did I wrong.
    – Akira
    Nov 30, 2017 at 8:53
  • @Akira I do not understand your comment. With the approach I suggested you will be reading the settings from a file into a container and the container itself won't tell anybody how it became filled. Nov 30, 2017 at 9:25

3 Answers 3

4

Some options:

  • An alias like using Settings = std::map<std::string, std::string> cuts down on typing that all out, but is still repeated
Settings readSettings() {
    Settings settings;

    for(std::string rawSetting; std::getline(settingsFile, rawSetting); )
        settings.insert(parseRawSettingIntoPair(rawSetting));

    return settings;
}
  • Direct construct the return value from a coroutine - still quite wordy
std::map<std::string, std::string> readSettings() {
    using coro_t = boost::asymmetric_coroutine<std::pair<std::string, std::string>>;
    coro_t::pull_type source([&](coro_t::push_type& sink) { 
        for(std::string rawSetting; std::getline(settingsFile, rawSetting); )
            sink(parseRawSettingIntoPair(rawSetting));
    });
    return { begin(source), end(source) };
}
  • Both of those ( because why not :P )
2
  • Thank your for your answer! I tested your second approach and it works very well. Regardless of your first approach is almost the same as @Ike's approach, combining it with your second approach makes it possible to change the container type from std::map to any other standard container type (including sequential containers) through one line (or through a template parameter) which is a very handy outcome.
    – Akira
    Nov 30, 2017 at 9:59
  • 1
    Yes, typedef and using (in this manner) have the same meaning.
    – Caleth
    Nov 30, 2017 at 10:07
3

You can just utilize typedefs.

typedef std::map<std::string, std::string> Settings;

Settings readSettings() {
    Settings settings;
    for(std::string rawSetting; std::getline(settingsFile, rawSetting); )
        settings.insert(parseRawSettingIntoPair(rawSetting));
    return settings;
}

... not exactly groundbreaking material, but does the job. As for redundancy, it's all syntax as I see it -- not like anything which is prone to bugs at runtime.

Trying to make code as easy to read and write as possible should not precede making sure it's reliable and well-tested so that you don't have to bother with constantly changing it and re-reading it if you ask me, but the simple typedef example above is a quick and simple solution to ease the effort without spending much time thinking about it or requiring the compiler to deduce the return type from the implementation with auto (which would require the implementation to be visible to callers as you pointed out).

Finally it makes it easy to change Settings to a different type that has the same interface requirements if you ever need it -- only requires changing one line of code.

9
  • Are you sure that Settings readSettings() { Settings settings; ... constitutes a reduction of repetition? Nov 29, 2017 at 15:41
  • Definitely not repetition of logic! It's why I made the little commentary that DRY applied at a syntactical level (purely for readability/writability) is something that probably isn't worth exhausting so much as making sure things test correctly and function properly. That said, a type alias using typedef or using can serve as a practical shorthand that doesn't require much thought and has benefits that make the code easier to change later.
    – user204677
    Nov 29, 2017 at 15:49
  • 1
    It will make changing between std::map and std::unordered_map easier. Meanwhile, it will not make changing to std::vector<std::pair<...>> any easier, because a vector requires a push_back or emplace_back. I would say that this question highlights the idiosyncraticity of the design of STL.
    – rwong
    Nov 30, 2017 at 9:14
  • 1
    @Ike, I understand your reasoning and yes, using associative containers for key-value pairs is a rational decision. I accepted Caleth's answer because it really eliminate the repetition of type specifiaction. Anyway your answer is noteworthy too because it points out whether the effort is really worth it.
    – Akira
    Nov 30, 2017 at 10:24
  • 1
    @Akira Oh cheers! Sorry, I was responding to rwong about the design of the STL and got a bit ranty there. I often find the STL to be very nice but never cared for the fact that they deliberately make even the interfaces or two sequences different and omitted certain methods solely for the sake of shielding programmers from calling inefficient methods over polymorphism,
    – user204677
    Nov 30, 2017 at 10:43
2

Just for completeness, here is an approach that does not return the result as an STL collection. Instead, it passes data into a callback. This may or may not satisfy your project's requirement; it is mentioned here just for completeness.

Two caveats.

Firstly, the signature for the std::function is uglier than the std::map, with nested angled brackets (template), parentheses (the arguments to the function), and a void for no benefit.

Secondly, if the person who provides the callback does something stupid inside the callback (such as trying to modify the collection or manipulate the same class), undefined behavior can be triggered. In other words, this callback approach is fragile unless all users of the code are well-informed.

Once again, this is provided for completeness' sake, not as a recommended practice.

void readSettings(std::function<
    void(const std::string& key, const std::string& value)> func)
{
    for(std::string rawSetting; std::getline(settingsFile, rawSetting); )
    {
        std::string key;
        std::string value;
        std::tie(key, value) = parseRawSettingIntoPair(rawSetting);
        func(key, value);
    }
}

The main motivation for writing this answer is if the caller (consumer of the data) is already known for refusing to use the same STL collection type as the function. In this case, the caller's insistence in using a different type means glue code is needed, so the function's author might as well turn itself into the glue code.

Such situation happens when the two side of code are maintained by different camps of developers.

1
  • Your argument is to be considered too, so your answer really completes the set of answers. Thank you for that!
    – Akira
    Nov 30, 2017 at 10:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.