Find Iterator in a std::vector

Question

I'm having a problem, i'm trying to create a System to read and write .ini files, but i'm stuck at how to correctly add the sections/keys. I thought i could add them as objects to a std::vector, so i wrote the following snippet (i removed code that is irrelevant):

// .h

class Ini
{
public:
    class IniSection
    {
        friend class Ini;

    private:
        IniSection(Ini *ini, std::string section);
        ~IniSection();

    private: 
        Ini *pIni;
        std::string sectionName;
    };

    vector<IniSection *>::iterator Ini::FindSection(std::string section);

    IniSection *AddSection(std::string name);

private:
    vector<IniSection *> Sections;
};

typedef Ini::IniSection IniSection;



// .cpp


IniSection::IniSection(Ini *ini, std::string section) : pIni(ini), sectionName(section)
{

}

vector<IniSection *>::iterator Ini::FindSection(std::string section)
{
    IniSection tempSection(NULL, section);

    return find(Sections.begin(), Sections.end(), tempSection); // ERROR
}

IniSection *Ini::AddSection(std::string name)
{
    vector<IniSection *>::const_iterator iter = FindSection(name);

    if (iter == Sections.end())
    {
        IniSection *newSection = new IniSection(this, name);
        Sections.push_back(newSection);
        return newSection;
    }
    else
        return *iter;
}

As you can see i marked a line with

// ERROR

that's because when i try to build it, i get this error:

Error C2679 binary '==' : no operator found which takes a right-hand operand of type 'const Ini::IniSection' (or there is no acceptable conversion)

It occurs in the algorithm library. So i guess the way i try to get the iterator is wrong, so maybe someone is able to help me out. I couldn't find something on the Internet that suited me. Thanks in advance!

Possible duplicate of [How to use std::find/std::find\_if with a vector of custom class objects?](https://stackoverflow.com/questions/6939129/how-to-use-stdfind-stdfind-if-with-a-vector-of-custom-class-objects) — haronaut, Oct 25 '17 at 22:02
You should provide a custom `operator ==` to compare `IniSection*` entries for equality. Also, `tempSection` is of type `IniSection`, but `Sections` contains `IniSection *` entries (pointers). — cantordust, Oct 25 '17 at 22:04
Unrelated: You might find `std::map` a useful replacement for `std::vector` here. — user4581301, Oct 25 '17 at 22:10
`std::map` -- where the key is the section name. All that linear search logic and pointers basically disappear if set up this way. — PaulMcKenzie, Oct 25 '17 at 22:39
@SupaDupa -- You are associating a section name with an `IniSection`. The keyword is *associating*. The container that fits this description is a `std::map` or `std::unordered_map`. A `std::vector` of pointers is IMO clumsy and error prone for this job. — PaulMcKenzie, Oct 25 '17 at 22:41
@SupaDupa [See this small example](http://coliru.stacked-crooked.com/a/dd6385e4bb60e44f). Note that adding a section requires no check for the name, as the map's `insert()` function will either insert a new item if the key doesn't exist, or will skip inserting if it does exist. In any of those scenarios, a std::pair is returned describing the outcome of the `insert()` call. — PaulMcKenzie, Oct 25 '17 at 23:07

score 1 · Answer 1 · answered Oct 25 '17 at 22:11

1

You need to add an operator== to your class IniSection which answers the question "are 2 instances of IniSection equal?". Something like this:

bool operator==(const IniSection& lhs, const IniSection& rhs)
{
   return (lhs.sectionName == rhs.sectionName);
}

answered Oct 25 '17 at 22:11

seladb

852
1
13
29

I tried to implement your code, but it gives me an error saying it has too many parameters for this operator function :/ – SupaDupa Oct 25 '17 at 22:34

cantordust · Answer 2 · 2017-10-26T10:22:35.163

There are several issues with the above code. As suggested, you need to define an operator == for IniSection in order for std::find to be able to compare the entries in your Sections vector. However, there is another problem: your FindSection is somewhat useless because you cannot test whether the iterator returned points at Sections::end() because Sections is a private member. If I may suggest, you need to add something like a hasSection method which returns bool, and test for the presence or absence of sections using that. I took the liberty of modifying the code to illustrate the point, feel free to change it as you see fit.

main.hpp

#ifndef MAIN_HPP
#define MAIN_HPP

#include <iostream>
#include <vector>
#include <string>
#include <algorithm>

class Ini
{
public:
    class IniSection
    {
        friend class Ini;

        Ini& pIni;
        std::string sectionName;
    public:
        IniSection(Ini& ini, const std::string& section);

        friend bool operator == (const IniSection& _this, const IniSection& _other)
        {
            return _this.sectionName == _other.sectionName;
        }
    };

    std::vector<IniSection>::iterator FindSection(const std::string& section);

    std::vector<IniSection>::iterator AddSection(const std::string& section);

    bool hasSection(const std::string& section);

    std::vector<std::string> sectionNames();

private:
    std::vector<IniSection> Sections;
};

typedef Ini::IniSection IniSection;


#endif // MAIN_HPP

main.cpp

#include "main.hpp"


IniSection::IniSection(Ini& ini, const std::string& section)
    :
      pIni(ini),
      sectionName(section)
{}

std::vector<IniSection>::iterator Ini::FindSection(const std::string& section)
{
    IniSection tempSection(*this, section);
    return std::find(Sections.begin(), Sections.end(), tempSection);
}

std::vector<IniSection>::iterator Ini::AddSection(const std::string& section)
{
    std::vector<IniSection>::iterator iter = FindSection(section);

    if (iter == Sections.end())
    {
        Sections.emplace_back(*this, section);
        return Sections.end() - 1;
    }
    else
    {
        return iter;
    }
}

bool Ini::hasSection(const std::string& section)
{
    return FindSection(section) != Sections.end();
}

std::vector<std::string> Ini::sectionNames()
{
    std::vector<std::string> names;
    for (const auto& s : Sections)
    {
        names.push_back(s.sectionName);
    }
    return names;
}

int main()
{
    Ini ini;

    ini.AddSection("Section1");
    ini.AddSection("Section2");
    ini.AddSection("Section3");

    if (ini.hasSection("Section1"))
    {
        std::cout << "Ini contains section 1!\n";
    }

    if (!ini.hasSection("Section4"))
    {
        std::cout << "Adding section 4...\n";
        ini.AddSection("Section4");
    }

    std::cout << "Ini contains the following sections:\n";
    for (const auto& s : ini.sectionNames())
    {
        std::cout << "\t" << s << "\n";
    }

    return 0;
}

This prints:

Ini contains section 1!
Adding section 4...
Ini contains the following sections:
    Section1
    Section2
    Section3
    Section4

Also, as suggested in the comments, std::unordered_map is a great performer and quite suitable for such scenarios.

P.S. You may have noticed that I replaced raw pointers with objects. STL containers like std::vector handle the allocation and deallocation internally, so there is no need to use raw pointers.

EDIT

I have modified the code from pastebin, but there are some things missing (I'm not sure what type IniPath is - maybe a std::string?). There is a lot of room for improvement, for example, you can add a couple of lines to getSection to add a section automatically if it doesn't exist (so you don't have to check for existence with hasSection first). As mentioned before, you could greatly benefit from using std::unordered_map as insertion, deletion and lookup are all amortised constant time. With std::vector, finding and retrieving an element would be linear time. Another thing I am not clear about is why you need to store a reference (or a pointer in your original code) to the parent object (such as IniSection& inside IniKey). As far as I can tell, you are not using these. You also don't need to implement your own destructors as long as you are not storing raw pointers.

Disclaimer: I have not compiled and tested this code.

P.S. You might want to post this on CodeReview for further improvements.

main.hpp

#include <vector>
#include <string>
#include <fstream>
#include <algorithm>

class Ini
{

public:

    Ini();
    ~Ini();

    bool Load(std::string path);

    void Load(std::istream& input);

    bool Save(std::string path);

    void Save(std::ostream& output);

    void Create(std::string path);

    class IniSection
    {
        friend class Ini;

        Ini& pIni;
        std::string sectionName;

    public:
        IniSection(Ini& ini, const std::string& section);
        ~IniSection();

        friend bool operator == (const IniSection& _this, const IniSection& _other)
        {
            return _this.sectionName == _other.sectionName;
        }


        std::string GetSectionName();

        class IniKey
        {
            friend class IniSection;

            IniSection& pSection;
            std::string keyName;
            std::string keyValue;
            std::string commentValue;

        public:
            IniKey(IniSection& section, const std::string& key);
            ~IniKey();

            friend bool operator == (const IniKey& _this, const IniKey& _other)
            {
                return _this.keyName == _other.keyName;
            }

            void SetValue(std::string value);

            std::string GetValue();

            std::string GetKeyName();

            void AddComment(std::string comment);

            std::string GetComment();
        };

        void RemoveAllKeys();

        std::vector<IniKey>::iterator FindKey(const std::string& key);

        std::vector<IniKey>::iterator AddKey(const std::string& key);

        std::vector<IniKey>::iterator GetKey(const std::string& key);

        bool hasKey(const std::string& key);

        std::string GetKeyValue(const std::string& key);

        void SetKeyValue(const std::string& key, const std::string& value);

    private:
        std::vector<IniKey> Keys;
    };

    void RemoveAllSections();

    std::vector<IniSection>::iterator FindSection(const std::string& section);

    std::vector<IniSection>::iterator AddSection(const std::string& section);

    std::vector<IniSection>::iterator GetSection(const std::string& section);

    bool hasSection(const std::string& section);

    std::string GetKeyValue(std::string section, std::string key);

    void SetKeyValue(std::string section, std::string key, std::string value);

private:
    std::vector<IniSection> Sections;
};

typedef Ini::IniSection IniSection;
typedef IniSection::IniKey IniKey;

main.cpp

#include "main.hpp"

bool Ini::Load(std::string path)
{
    std::ifstream input;

    input.open(path.c_str(), std::ios::binary);

    if (!input.is_open())
        return false;

    Load(input);

    input.close();

    return true;
}

void Ini::Load(std::istream& input)
{
    std::vector<IniSection>::iterator section;
    std::string lineValue;

    enum
    {
        KEY,
        SECTION,
        COMMENT,
        OTHER
    };

    while (getline(input, lineValue))
    {
//      TrimLeft(lineValue);
//      TrimRight(lineValue, "\n\r");

        if (!lineValue.empty())
        {
            unsigned int type = OTHER;

            type = (lineValue.find_first_of("[") == 0 && (lineValue[lineValue.find_last_not_of(" \t\r\n")] == ']')) ? SECTION : OTHER;
            type = ((type == OTHER) && (lineValue.find_first_of("=") != std::string::npos && lineValue.find_first_of("=") > 0)) ? KEY : type;
            type = ((type == OTHER) && (lineValue.find_first_of("#") == 0)) ? COMMENT : type;

            switch (type)
            {
            case SECTION:
                section = AddSection(lineValue.substr(1, lineValue.size() - 2));
                break;
            case KEY:
                {
                    size_t equalSpot = lineValue.find_first_of("=");
                    std::string keyName = lineValue.substr(0, equalSpot);
                    std::string keyValue = lineValue.substr(equalSpot + 1);
                    std::vector<IniKey>::iterator key = section->AddKey(keyName);
                    key->SetValue(keyValue);
                    break;
                }

            default:
                break;
            }
        }
    }
}

void Ini::Create(std::string path)
{
    std::fstream file;

    file.open(path, std::fstream::out);

    file.close();
}

bool Ini::Save(std::string path)
{
    std::ofstream output;

    output.open(path.c_str(), std::ios::binary);

    if (!output.is_open())
    {
        output.close();

        return false;
    }

    Save(output);

    output.close();

    return true;
}

void Ini::Save(std::ostream& output)
{
    std::string section;
    std::vector<IniSection>::iterator iter1;

    for (iter1 = Sections.begin(); iter1 != Sections.end(); iter1++)
    {
        section = "[" + iter1->GetSectionName() + "]";

        output << section << "\r\n";

        std::vector<IniKey>::iterator iter2;

        for (iter2 = iter1->Keys.begin(); iter2 != iter1->Keys.end(); iter2++)
        {
            std::string comment = "# " + iter2->GetComment();

            if (comment != "# ")
                output << comment << "\r\n";

            std::string key = iter2->GetKeyName() + "=" + iter2->GetValue();

            output << key << "\r\n";
        }

        output << "\r\n";
    }
}

std::string Ini::GetKeyValue(std::string section, std::string key)
{
    if (hasSection(section))
    {
        auto s = GetSection(section);
        if (s->hasKey(key))
        {
            return s->GetKey(key)->GetValue();
        }
    }
    return std::string();
}

void Ini::SetKeyValue(std::string section, std::string key, std::string value)
{
    if (hasSection(section))
    {
        auto s = GetSection(section);
        if (s->hasKey(key))
        {
            s->GetKey(key)->SetValue(value);
        }
    }
}

// IniSection -----------------------------------------------------------------------------------

IniSection::IniSection(Ini& ini, const std::string& section) : pIni(ini), sectionName(section)
{

}

void Ini::RemoveAllSections()
{
//  std::vector<IniSection *>::iterator iter;

//  for (iter = Sections.begin(); iter != Sections.end(); iter++)
//      delete *iter;

    Sections.clear();
}

std::vector<IniSection>::iterator Ini::FindSection(const std::string& section)
{
    IniSection tempSection(*this, section);

    return std::find(Sections.begin(), Sections.end(), tempSection);
}

std::vector<IniSection>::iterator Ini::AddSection(const std::string& section)
{
    std::vector<IniSection>::iterator iter = FindSection(section);

    if (iter == Sections.end())
    {
        Sections.emplace_back(*this, section);

        return Sections.end() - 1;
    }
    else
        return iter;
}

std::vector<IniSection>::iterator Ini::GetSection(const std::string& section)
{
    return FindSection(section);
}

std::string IniSection::GetSectionName()
{
    return sectionName;
}

std::string IniSection::GetKeyValue(const std::string& key)
{
    if (hasKey(key))
    {
        return GetKey(key)->GetValue();
    }
    return std::string();
}

void IniSection::SetKeyValue(const std::string& key, const std::string& value)
{
    if (hasKey(key))
        GetKey(key)->SetValue(value);
}

// IniKey -----------------------------------------------------------------------------------

void IniSection::RemoveAllKeys()
{
    //  std::vector<IniKey *>::iterator iter;

    //  for (iter = Keys.begin(); iter != Keys.end(); iter++)
    //      delete *iter;

    // std::vector manages the allocations automatically
    // as long as you are not using raw pointers
    Keys.clear();
}

std::vector<IniKey>::iterator IniSection::FindKey(const std::string& key)
{
    IniKey tempKey(*this, key);
    return std::find(Keys.begin(), Keys.end(), tempKey);
}

std::vector<IniKey>::iterator IniSection::AddKey(const std::string& key)
{
    if (hasKey(key))
    {
        return GetKey(key);
    }
    else
        return Keys.insert(Keys.end(), {*this, key});
}

bool IniSection::hasKey(const std::string& key)
{
    return FindKey(key) != Keys.end();
}

void IniKey::SetValue(std::string value)
{
    keyValue = value;
}

std::string IniKey::GetValue()
{
    return keyValue;
}

std::string IniKey::GetKeyName()
{
    return keyName;
}

std::vector<IniKey>::iterator IniSection::GetKey(const std::string& key)
{
    if (hasKey(key))
        return GetKey(key);

    return Keys.end();
}

bool Ini::hasSection(const std::string& section)
{
    return FindSection(section) != Sections.end();
}

void IniKey::AddComment(std::string comment)
{
    commentValue = comment;
}

std::string IniKey::GetComment()
{
    return commentValue;
}


int main()
{
    // How i want to use it:
    // Write:
    Ini ini;

    ini.Create(IniPath);

    ini.Load(IniPath);

    // Check if "Test" section exists and add it if it doesn't
    if (!ini.hasSection("Test"))
    {
        ini.AddSection("Test");
    }

    auto secTest(ini.GetSection("Test"));
    secTest->AddKey("Key1")->SetValue("KeyValue1");
    secTest->GetKey("Key1")->AddComment("This is a Test");

    ini.Save(IniPath);

    // Read:
    Ini ini1;

    ini1.Load(IniPath);

//  IniSection *Section = ini.GetSection("Test");

    if (ini1.hasSection("Test"))
    {
        if (ini1.GetSection("Test")->hasKey("Key1"))
        {
            std::string keyValue = ini.GetSection("Test")->GetKeyValue("Key1");
        }
    }

    return 0;

}

Okay it seems like you understand what you are doing, but this is completely different from what i did, so it's messing with all my other stuff and now i just have errors all over the place... I know i'm asking for much, but could you tell me what i have to do in order to get it working, i'm kind of new to using vectors and such, i always used arrays, but it seemed the wrong thing to do here. I posted all of my code so far here: https://pastebin.com/w2yqVhec — SupaDupa, Oct 25 '17 at 23:20
I have modified the code from pastebin, but it needs further work (there are some redundant member functions and so on). I don't have the time to do this now, but maybe if you post it on CodeReview you might get good suggestions. — cantordust, Oct 26 '17 at 00:46
first of all thanks for all your efort, but im still having a small problem now: [link](https://imgur.com/dTyz4lW) I'm not using C++11, if that's of importance... — SupaDupa, Oct 26 '17 at 09:55
Try replacing it with `return Keys.insert(Keys.end(), IniKey(*this, key));` — cantordust, Oct 26 '17 at 10:16
It worked, sadly now i get this error when trying to build: _Error C2582 'operator =' function is unavailable in 'Ini::IniSection::IniKey' Project C:\Program Files (x86)\Microsoft Xbox 360 SDK\include\xbox\xutility 2514_ Any idea how to fix this? I added the Operator to my project, but i'm not sure what it should return `IniKey& IniKey::operator=(IniKey &){}` — SupaDupa, Oct 26 '17 at 11:10
That would be because you are storing a reference to `IniSection` as a data member. References cannot be reseated, so as a consequence your default `operator =` is disabled. But this would only happen if you try to reassign a key to another key (`iniKey1` = `iniKey2`). Is that the case? If you want that functionality, the easiest thing to do would be to either not store a reference at all (I think you don't need to anyway) or to store pointers, as in your original code. And I again strongly recommend that you consider `std::unordered_map` as your container. — cantordust, Oct 26 '17 at 21:25

Find Iterator in a std::vector

2 Answers2

main.hpp

main.cpp

Linked