Pointers and ownership

Want to discuss changes to the UOX3 source code? Got a code-snippet you'd like to post? Anything related to coding/programming goes here!
Post Reply
punt
VIP
Posts: 244
Joined: Wed Mar 24, 2004 7:46 pm
Has thanked: 0
Been thanked: 9 times

Pointers and ownership

Post by punt »

I tore my rotator cuff in my shoulder, so going to be out of commission for a while. But I wanted to document some thoughts on pointers, and ownership.

Modern C++ has smart pointers, the two most common are std::unique_ptr<T> and std::shared_ptr<T>

They where created to resolve two issues with pointers: 1. leaks, and 2. Dangling references.
Leak is where a pointer is created (say with a new), and then not properly deleted. So that object "leaks" (and it could be more then memory, it could be file descriptors, sockets, etc). These smart pointers ensure that the object is deleted when there are no more references to them. So one shouldn't every need to worry about a delete being preformed.

Dangling is where one references a pointer, that has been deleted and the object no longer exists. One can use a shared pointer for this. However, there is cost to pay for shared pointers, the overhead of reference counting.

When one gets a pointer, does one need to also pass ownership with it? That is the design question to ask. If the answer is yes (I need this item to retain ownership of the pointer, responsible for keeping it active), then one should pass a shared pointer (assume others will use it as well). But say you have a store of pointers, that you create on initialization, and then delete on termination. When routines need to access those pointers, they don't need "ownership" of the pointer, they just need access to it. In that case one doesn't need to pass it the smart pointer (the store should have still been created with smart pointers, but std::unique_ptr<T>), but the raw pointer below (with the .get()) method). This saves overhead, and makes one understand their design (is it an access or ownership issue).

I have been guilty of mixing ownership with access, and it can cost one a lot of unnecessary overhead.

The second thing to start considering, is does on really need a pointer, or just a reference? Once one has a store (as described above), does one really need to have a pointer? Can one just pass back a reference to the object, since one knows it will be persisted throughout the program life.

I probably haven't worded this well, but felt it a common trap, one I fall into all the time. So think about ownership of an object separate from use of an object.
User avatar
Xuri
Site Admin
Posts: 3704
Joined: Mon Jun 02, 2003 9:11 am
Location: Norway
Has thanked: 48 times
Been thanked: 8 times
Contact:

Post by Xuri »

Ouch! Sounds painful! How'd that happen? D:

Thanks for the run-through on pointers - I don't have much to contribute to the topic as part of a design discussion, but it sounds sensible!
-= Ho Eyo He Hum =-
punt
VIP
Posts: 244
Joined: Wed Mar 24, 2004 7:46 pm
Has thanked: 0
Been thanked: 9 times

Post by punt »

So what does that mean. I figure an example is best. The base format of many of UOX3 text data is the following format:
[section name or some identifier]
{                                 <<<<< Section start
       key = value          <<<<< some key, then = to some value (value can be numbers, string, etc
}                                <<<< end section
So how would be build some methods to easily read and access this type of data?
I have three files to generically read this type of data. These take the concept of ownership of pointers, and access of pointers.

The first is KeyValue. This represents the "key = value" line
//Copyright © 2021 Charles Kerr. All rights reserved.
#ifndef KeyValue_hpp
#define KeyValue_hpp

#include <string>
#include <tuple>

using namespace std::string_literals;
class SecContainer ;
/*
 This represents a line that is formatted to have a "key" and a "value".
 Example:  key = value
 */

class KeyValue {
private:
    std::string _line ;     // Original line
   
    std::string _key  ;     // The "key" portion
    std::string _upper_key; // The uppercase version of the key
   
    std::string _value ;    // The value porition
   
protected:
    // Debugging/Error information
    std::string _origin ;
    std::string _linenumber ;
    friend class SecContainer ;
   
public:
    KeyValue(const std::string& input="") ;
    void operator=(const std::string& input);
    bool operator==(const KeyValue &rhs) const ;
    // A keyvalue without a key is san "invalid" key.  So we allow
    // one to check if we are true/false for that.
    explicit operator bool() const { return !_key.empty();}

   
    auto original() const {return _line;}
    auto description() const {return _key + " = "s + _value;}
   
    // The accessors for the values ;
    auto key() const {return _key;}
    auto upperKey() const {return _upper_key;}
    auto value() const {return _value;}
   
    std::tuple<int,int> number() const ;
   
    bool boolean() const ;
   
    auto origin() const {return _origin;}
    auto lineNumber() const {return _linenumber;}
    auto source() const {return "Line: "s + _linenumber +" Input: "s + _origin;};

};


#endif /* KeyValue_hpp */
//Copyright © 2021 Charles Kerr. All rights reserved.

#include "KeyValue.hpp"
#include "StringUtility.hpp"
#include <stdexcept>
//=====================================================================
KeyValue::KeyValue(const std::string& input) :_origin(),_linenumber(),_key(),_value(),_line(){
    _line = input ;
    if (!_line.empty()){
   
        std::tie(_key,_value) = strutil::split(_line,"=");
        if (_key.empty()){
            throw std::runtime_error("Key is empty"s);
        }
        _upper_key = strutil::upper(_key);
    }
}

//=====================================================================
void KeyValue::operator=(const std::string& input){
    _line = input ;
    std::tie(_key,_value) = strutil::split(_line,"=");
    if (_key.empty()){
        throw std::runtime_error("Key is empty"s);
    }
    _upper_key = strutil::upper(_key);

}

//=====================================================================
bool KeyValue::operator==(const KeyValue &rhs) const {
    return (_key == rhs._key) && (_value == rhs._value) ;
}

//=====================================================================
std::tuple<int,int> KeyValue::number() const {
    auto first = ""s ;
    auto second = ""s ;
   
    std::tie(first,second) = strutil::split(strutil::simplify(_value)," ");
    auto fnum = 0 ;
    auto snum = 0 ;
    try {
        fnum = std::stoi(first,nullptr,0) ;
        snum = fnum;
        snum = std::stoi(second,nullptr,0);
    }
    catch (...){
       
    }
    return std::make_tuple(fnum,snum);
}

//=====================================================================
bool KeyValue::boolean() const {
    auto first = 0i;
    auto second = 0i;
    std::tie(first,second) = this->number();
    if (first != 0){
        return true ;
    }
    return false ;
}
The second is Section.hpp. This represents the section (the name of the section, and a vector of all key values
//Copyright © 2021 Charles Kerr. All rights reserved.
#ifndef Section_hpp
#define Section_hpp

#include <vector>
#include <string>
#include <memory>

#include "StringUtility.hpp"
#include "KeyValue.hpp"
using namespace std::string_literals;

class SecContainer;
class Section {

private:
    std::vector<std::unique_ptr<KeyValue>> _keyvalues ;
    std::string _section ;
    std::string _upper_section ;

protected:
    std::string _origin ;
    std::string _start_line ;
    std::string _end_line ;
    // This method moves the keyvalue, so the keyvalue is invalid for the calling parent upon return
    void addKeyValue(std::unique_ptr<KeyValue> &&keyvalue);
    friend class SecContainer;
public:
    Section(const std::string &input="") ;
    // A section without a name is potentially an "invalid" section.  So we allow
    // one to check if we are true/false for that.
    explicit operator bool() const { return !_section.empty();}
   
    // We have a container of unique pointers, so we can't be copied anymore
    Section(const Section&) = delete ;
    Section & operator=(const Section &) = delete ;
   
    std::string name() const {return _section;}
    void name(const std::string &value){_section = value;_upper_section = strutil::upper(value);}
    std::string upper_name() const { return _upper_section;}
   
    std::size_t size() const {return  _keyvalues.size();}
   
    /*
     These routines are passing back access to the keyvalues, but not ownership
     Thus, this Section Object has to remain active as long as anyone is holding
     on to these references.  One could pay the cost of returning shared_ptr, but
     we know our use case there is no need to pay that overhead with shared_ptr
     */

    std::vector<std::unique_ptr<KeyValue>> & keyValues() ;
    std::vector<KeyValue*> keyValues(const std::string &key);
    KeyValue* keyValue(const std::string &key);
   
    std::string description() const ;
   
    std::string span() const { return _start_line + " - "s + _end_line;}
    std::string origin() const { return _origin;}
    std::string source() const {return "Span: "s + _start_line + " - "s + _end_line +" Input: "s + _origin; }
   
};

#endif /* Section_hpp */
//Copyright © 2021 Charles Kerr. All rights reserved.

#include "Section.hpp"
#include "KeyValue.hpp"
#include <algorithm>
#include <sstream>
//=====================================================================
Section::Section(const std::string &input):_section(input){
    _upper_section = strutil::upper(input);
}

//=====================================================================
// This method moves the keyvalue, so the keyvalue is invalid upon return
void Section::addKeyValue(std::unique_ptr<KeyValue> &&keyvalue){
    _keyvalues.push_back(std::move(keyvalue));
}

//=====================================================================
std::vector<std::unique_ptr<KeyValue>> & Section::keyValues() {
    return _keyvalues;
}

//=====================================================================
std::vector<KeyValue*> Section::keyValues(const std::string &key) {
    auto match = strutil::upper(key);
    std::vector<KeyValue*> rvalue ;
    for (auto &entry : _keyvalues){
        if (entry->upperKey()== match){
            rvalue.push_back(entry.get());
        }
    }
    return rvalue ;
}

//=====================================================================
std::string Section::description() const {
    std::stringstream buffer ;
    buffer <<"[" << _section<<"]\n{\n";
    for (auto &e:_keyvalues){
        buffer <<"\t"<<e->description()<<"\n";
    }
    buffer <<"}" <<std::endl;
    return buffer.str() ;
}
And lastly, a class to contain a group sections (from a single file, or from all files in a directory)
//Copyright © 2021 Charles Kerr. All rights reserved.
#ifndef SecContainer_hpp
#define SecContainer_hpp

#include <string>
#include <map>
#include <memory>
#include <filesystem>
#include "Section.hpp"
class SecContainer {
private:
    std::map<std::string,std::unique_ptr<Section>> _sections;
   
    // Create a section and add it to the storage
    // Ownership is NOT returned, just access to the section
    Section* createSection(const std::string &name, const std::string& origin, const std::string &startline);
   
    // Create and add a KeyValue to a section
    void addKeyValue(const std::string &line, Section* section, const std::string& origin, const std::string &linenumber);

    void processDirEntry(const std::filesystem::path &path, const std::string &extension);
public:
    SecContainer(const std::string &filepath="");
    // We have a container of unique pointers, so we can't be copied anymore
    SecContainer(const SecContainer&) = delete ;
    SecContainer & operator=(const SecContainer &) = delete ;

    void load(const std::string &filepath);
    void loadDir(const std::string &directory, bool recursive = true, const std::string &extension=".cfg");
   
    std::map<std::string,std::unique_ptr<Section>> & sections() {return _sections;} ;
    Section* section(const std::string& name) ; // Remember that section names are searched upper case!
   
    void clear() { _sections.clear();}
    std::size_t size() const { return _sections.size();}
};


#endif /* SecContainer_hpp */
//Copyright © 2021 Charles Kerr. All rights reserved.

#include "SecContainer.hpp"
#include "StringUtility.hpp"
#include <algorithm>
#include <fstream>
#include <iostream>
#include <stdexcept>
//=====================================================================
SecContainer::SecContainer(const std::string &filepath){
    if (!filepath.empty()){
        load(filepath);
    }
}

//=====================================================================
Section* SecContainer::createSection(const std::string &name, const std::string& origin, const std::string &startline){
    auto sec = std::make_unique<Section>(name);
    sec->_origin = origin;
    sec->_start_line = startline;
    auto rvalue = sec.get();
    _sections.insert_or_assign(sec->upper_name(), std::move(sec));
    return rvalue ;
}

//=====================================================================
void SecContainer::addKeyValue(const std::string &line, Section* section, const std::string& origin, const std::string &linenumber){
    auto keyvalue = std::make_unique<KeyValue>(line);
    keyvalue->_origin = origin;
    keyvalue->_line = linenumber ;

    section->addKeyValue(std::move(keyvalue));
}
//=====================================================================
void SecContainer::load(const std::string &filepath){
    const auto bufsize = 2048;
    char buffer[bufsize];
    // Define the three states of searching in the file
    enum searchstate {header,startsection,endsection} ;
    auto state = searchstate::header;
    std::ifstream input(filepath);
    if (!input.is_open()){
        return;
    }
    auto linecount = 1 ; // We keep the line of the file
    Section *sec = nullptr ;
    while (!input.eof()){
        input.getline(buffer, bufsize-1);
        buffer[input.gcount()]=0 ; // Null terminate the buffer
        // We want to condition the line
        auto inputline = strutil::trim(strutil::removeTrailing(buffer,"//"));

        if (!inputline.empty()){
            switch (state) {
                case header:
                {
                    auto name = strutil::contents(inputline, "[", "]");
                    if (name.empty()){
                        throw std::runtime_error("Invalid data '"s + inputline +"' on line "s + std::to_string(linecount)+" File: "s+filepath);
                    }
                    sec = createSection(name, filepath, std::to_string(linecount));
                    state = startsection ;
                }
                    break;
                case startsection:
                {
                    if (inputline!= "{"){
                        if (inputline[0]=='['){
                            throw std::runtime_error("Section name '"s + inputline+"' found while looking for section contents. Line: "s+std::to_string(linecount)+" File: "s + filepath);
                        }
                        else if (inputline[0]=='}'){
                            throw std::runtime_error("End Section found '"s + inputline+"' found while looking for section contents. Line: "s+std::to_string(linecount)+" File: "s + filepath);

                        }
                        else {
                            throw std::runtime_error("Invalid data '"s + inputline +"' on line "s + std::to_string(linecount)+" File: "s+filepath);

                        }
                    }
                    state = endsection;
                }
                    break;

                case endsection:
                {
                    if (inputline[0] == '['){
                        throw std::runtime_error("Section header found before end of section at line "s+std::to_string(linecount)+ " File: "s+filepath);
                    }
                    else if (inputline[0]== '{'){
                        throw std::runtime_error("Start of section found before end of section at line "s+std::to_string(linecount) + " File: "s + filepath);
                    }
                    else if (inputline[0] != '}'){
                        addKeyValue(inputline, sec, filepath, std::to_string(linecount));
                    }
                    else {
                        sec->_end_line = std::to_string(linecount);
                        state = header;
                       
                    }
                }
                   
                    break;
            }
        }
        linecount++;
    }
    if (state != header){
        sec->_end_line = std::to_string(linecount);
        throw std::runtime_error("File ended while processing section. File: "s + filepath);
    }
}
//=====================================================================
void SecContainer::loadDir(const std::string &directory, bool recursive, const std::string &extension){
    auto path = std::filesystem::path(directory) ;
    if (std::filesystem::is_directory(path)){
        if (recursive){
            for (auto &entry: std::filesystem::recursive_directory_iterator(path)){
                processDirEntry(entry, extension);
            }
        }
        else {
            for (auto &entry: std::filesystem::directory_iterator(path)){
                processDirEntry(entry, extension);
            }
        }
    }
}
//=====================================================================
void SecContainer::processDirEntry(const std::filesystem::path &path, const std::string &extension){
    if (std::filesystem::is_regular_file(path)){
        if (path.extension().string() == extension){
            load(path.string());
        }
       
    }
}
//=====================================================================
Section* SecContainer::section(const std::string& name){
    auto iter = _sections.find(name) ;
    if (iter == _sections.end()){
        return nullptr;
    }
    return iter->second.get() ;
}


These users thanked the author punt for the post:
Xuri
Post Reply