-2

I have created a config class which loads configuration from a config YAML. I have created vector containers for each type

// Pseudo Code
class config
{
private:
    std::vector<std::string> c_name;

public:
    config(yaml_file_path)
    {
        // Handles reading from yaml and loading data to c_name container.
        load(yaml_file_path);
    }

    std::vector<std::string> get_name()
    {
        return c_name;
    }
};

I use this object in other class to get the name config.

class loadConfig 
{
    config cfg(yaml_file_path);
    std::vector<std::string> name = cfg.get_name();
    // Further use of vector name like checks.
}

Question : What would be better?(as code practice and execution time and/or memory space)

  1. Using get_name() function in various places in code. OR
  2. Copying value in container, as I have done?
JeJo
  • 30,635
  • 6
  • 49
  • 88
ABHINAV RANA
  • 67
  • 1
  • 10
  • 2
    Even when creating only pseudo-code, please try to make it as syntactically correct as possible. Like what is that `` supposed to mean? What is the stray `}` doing? And `c_name` can't be used as a function. And there's a missing `}` at the end. – Some programmer dude Sep 29 '21 at 06:29

1 Answers1

3

What would be better?( as code practice and execution time and/or memory space)

Your get_name() function does the container copy each time of the call. This is much expensive and do not required as long as you do not want to modify it outside the class.

I would suggest having one/ both of the overloads instead, so that the compiler can choose upon the (non-const/ const)object you call:

// for the non-const `config` objects call
std::vector<std::string>& get_name() /* noexcept */ {
    return c_name;
}

// for the const `config` objects
const std::vector<std::string>& get_name() const /* noexcept */ {
    return c_name;
}

Now at caller, you can have

auto& name = cfg.get_name(); // non-const ref for further modifications

or

const auto& name = cfg.get_name(); // const-ref for read only purposes.

In both cases, you will not copy the container.


That being said, for the classes such as config, which has only one container as internal storage, my personal favorite would be making the class iterable by providing the begin and end overloads:

class config 
{
    std::vector<std::string> c_name;

public:    
    auto begin() noexcept { return c_name.begin(); }
    auto cbegin() const noexcept { return c_name.cbegin(); }
    auto end() noexcept { return c_name.end(); }
    auto cend() noexcept { return c_name.cend(); }
};

This makes the you write code such as>

config names;
for (auto& name : names) // or for (auto const& name : names)
{
    // ...do something with names
}
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • thank you for reply, so by adding `const` we are making it easier for compiler ? Also if i have multiple container in config class is it still advisable to have it return iterators ? I don't have just one container i have multiple which i further use to make key_pair and maps like `std::map key_pair ,std::pair range>` I even have a get function for maps like these. Do i do same `const` overload for same ? Basically i used these maps to check the range of any incoming value, hence the key_pair and maps. Advise as good approach ? – ABHINAV RANA Sep 29 '21 at 07:55