Einsatz von Makro hier sinnvoll? + Rückgabetyp



  • Erstmal ein bisschen Code vorweg, 2 Fragen hintendran.

    Header:

    class Scope {
    public:
        /* ... */
        decltype(variables_)::iterator getVariable(std::string const& name);
        decltype(pointers_)::iterator getPointer(std::string const& name);
        decltype(arrays_)::iterator getArray(std::string const& name);
    
    private:
        /* ... */
        Scope* parent_;
        std::map<std::string, ScriptData::ScriptVariable> variables_;
        std::map<std::string, ScriptData::ScriptDataPointer> pointers_;
        std::map<std::string, ScriptData::ScriptArray> arrays_;
    };
    

    Source:

    //-----------------------------------------------------------------------------------------------------------------------------
    #define getObject(cont) decltype(cont)::iterator ret;                   \
        Scope* curScope = this;                                             \
        do {                                                                \
            ret = curScope->(cont).find(name);                              \
            if (ret != curScope->(cont).end())                              \
                return ret;                                                 \
            curScope = parent_;                                             \
        } while (curScope != nullptr);                                      \
        return (cont).end();
    //-----------------------------------------------------------------------------------------------------------------------------
    decltype(variables_)::iterator Scope::getVariable(std::string const& name) {
        getObject(variables_);
    }
    //-----------------------------------------------------------------------------------------------------------------------------
    decltype(pointers_)::iterator Scope::getPointer(std::string const& name) {
        getObject(pointers_);
    }
    //-----------------------------------------------------------------------------------------------------------------------------
    decltype(arrays_)::iterator Scope::getArray(std::string const& name) {
        getObject(arrrays_);
    }
    //-----------------------------------------------------------------------------------------------------------------------------
    
    1. Ist das Makro hier zur Vermeidung von Codeduplizierung sinnvoll (und/oder) gibts noch ne bessere Lösung?
    2. Ich gebe einen Iterator zurück, bei Zeigern und Referenzen geht bei mir eigentlich immer sofort die Alarmglocke los. Wenn ich drüber nachdenke, finde ich das aber so deutlich schöner als all das andere Zeug was mir einfällt (zB. mit settern)

    EDIT: Im Code könnte hypothetisch ein Fehler stecken, bin noch nicht beim testen.



  • Hallo,

    nimm doch einfach ein Template:

    template<typename T>
    typename std::map<std::string, T>::iterator getObject<T>(const std::map<std::string, T> &scope_map)
    {
        typename std::map<std::string, T>::iterator ret;
        Scope* curScope = this;
        do {
            ret = scope_map.find(name);
            if (ret != scope_map.end())
                return ret;
            curScope = parent_;
        } while (curScope != nullptr);
        return scope_map.end();
    }
    

    Wegen den Variablen 'this' und *parent_' wirst du diese als Memberfunktion definieren müssen.

    Oder habe ich etwas übersehen?

    Edit: sehe gerade, daß 'const' nicht zu 'iterator' paßt, also entweder 'const' entfernen oder aber noch besser generell 'const_iterator' verwenden (deine Methode sucht ja nur!).



  • Beim Zurückgeben von Iteratoren auf privates Zeugs gehen bei mir sofort die Alarmglocken los.

    Und ... ich kann jetzt auf den 1. Blick nix erkennen was die Implementierung von getObject als Template verhindern würde.
    Du musst halt statt einer Referenz auf die map einen Member-Pointer übergeben...

    EDIT: Zu spät. Ich hätte wohl vor dem Schreiben meiner Antwort nochmal Refresh drücken sollen.

    EDIT2: Doch nicht zu spät, der Code von Th69 funktioniert ja gar nicht, weil er immer nur im ersten Objekt sucht, aber nie im Parent.

    EDIT3: Auch fragwürdig ist das Zurückgeben von "map.end()". Mit was soll der Aufrufer das denn vergleichen um rauszubekommen ob es "end" ist, wenn der Container private ist?



  • Wieso braucht dein User einen Iterator? Will er noch über andere Elemente iterieren 😕 (Referenz wäre sonst das Mittel der Wahl)

    Edit: Hustbaer, ➡

    curScope = curScope->parent_;
    

    ?
    Edit²: Oh, nein. War auch ein Parameter. Also nur parent_ .



  • @Sone
    Er hat im "nicht gefunden" Fall kein throw drinnen, also wohl eher ein Zeiger statt ner Referenz.



  • Hmmm. Ich will gar kein Handle zurückgeben, aber ich weil nicht im folgenden Code:

    auto var = Scope.getVariable("name"); // suchen
    var += 2;
    Scope.setVariable("name", var);  // suchen
    

    jedes mal bei Zugriff auf eine Variable alle Scopes danach durchsuchen, sonderm am besten nur einmal, deshalb hatte ich vor diese Regel zu brechen.

    EDIT: @hustbaer - Mir ist noch gar nicht aufgefallen, dass der Benutzer gar nicht mit dem Ende vergleichen kann 😃
    (EDIT2: woraufhin ich dann das Ende gleich mit angeboten hätte als Memberfunktion, sobald es mir aufgefallen wär... auch NICHT WIRKLICH schön)



  • Tim06TR schrieb:

    Hmmm. Ich will gar kein Handle zurückgeben, aber ich weil nicht im folgenden Code:

    auto var = Scope.getVariable("name"); // suchen
    var += 2;
    Scope.setVariable("name", var);  // suchen
    

    Was spricht gegen

    auto varptr = scope.getVariable("name"); // suchen
    *varptr += 2;
    

    ?
    Bzw. gleich

    scope.Variables["name"] += 2;
    

    Und halt für Fälle wo keine Exception im "gibt's nicht" Fall geworfen werden soll

    if (scope.Variables.Exists("name"))
        // ...
    // bzw.
    if (auto varptr = scope.Variables.Find("name"))
        // ...
    

    Oder auch

    class Scope
    {
    public:
        class VariableReference
        {
        private:
            // implementation defined - might store a pointer to Scope + Iterator into the map or whatever is necessary to avoid the second map lookup
        public:
            void Set(T const& t);
            T Get() const;
            explicit operator bool () const; // "is valid"
        };
    
        class ConstVariableReference { ... };
    
        VariableReference Variable(char const* name);
        ConstVariableReference Variable(char const* name) const;
    };
    
    void foo()
    {
        if (auto varref = scope.Variable("name"))
            varref.Set(varref.Get() + 2);
    }
    


  • @hustbaer: stimmt, hatte gestern nicht gesehen, daß immer auf eine andere Instanz zugegriffen wird, also muß hier dann tatsächlich ein Zeiger auf einen Member übergeben werden (aber immer noch besser als ein Makro zu benutzen).



  • Tim06TR schrieb:

    class Scope {
    public:
        /* ... */
        decltype(variables_)::iterator getVariable(std::string const& name);
        decltype(pointers_)::iterator getPointer(std::string const& name);
        decltype(arrays_)::iterator getArray(std::string const& name);
    	
    private:
        /* ... */
        Scope* parent_;
        std::map<std::string, ScriptData::ScriptVariable> variables_;
        std::map<std::string, ScriptData::ScriptDataPointer> pointers_;
        std::map<std::string, ScriptData::ScriptArray> arrays_;
    };
    

    Das klappt schomma nicht. Bevor du da für die Deklaration einer Funktion decltype(membervariable) verwenden kannst, muss die membervariable definiert sein. Du machst das hier falsch herum. So etwas darfst du nur innerhalb der Funktionsimplementierungen, also zwischen { und } machen, weil quasi

    struct foo
    {
      int get() const
      {
        return z; // was für ein z?
      }
      int z;
    };
    

    nur eine syntaktische Abkürzung für

    struct foo
    {
      inline int get() const;
      int z;
    };
    
    int foo::get() const
    {
      return z; // achso! ja, das z kenne ich! :)
    }
    

    ist.



  • Achja... man könnte es auch rekursiv implementieren.
    Müsste sogar wegoptimiert werden, Tail-Call und so.
    (Oder überseh' ich grad was?)



  • Yepp, wäre sogar noch schöner (da es sowieso eine Memberfunktion ist, wäre es designtechnisch besser, wenn die Funktion nur ihre eigene Instanz bearbeitet und nicht auch noch durch andere durchiteriert - außer den Aufruf des parent-Scopes natürlich).
    (nein, m.E. übersiehst du nichts 😉



    1. Ich nutze jetzt eine Funktionen Template getObject
    2. Ich maches es jetzt rekursiv
    3. Ich nutze eine Wrapper Klasse für Objektreferenzen mit get und set funktionen

    Das mit decltype war mir bewusst, ich habe public und private im Forum beim zusammenkopieren vertauscht.

    1. Ich registriere zurückgegebene Wrapper, die sich durch ihren Konstruktor über einen Funktionszeiger selbst aus der Liste wieder rausnehmen, sodass ich die Wrapperdaten bei Bedarf invalidieren kann.

    Ich weiß noch nicht ob ich für die Wrapper das kopieren etc verbiete oder ob ich sie Instanzen zählen lassen soll.

    EDIT: rekursiv ist sooo viel schöner!


Anmelden zum Antworten