Problem mit == operator overload



  • Hey, ich versuche aktuell einen Bug zu fixen der in der Kombination folgendes Headers und CPP Files auftritt.

    Header:

    #pragma once
    #include <chrono>
    class CFallingChar
    {
    public:
    	CFallingChar();
    
    	int GetSpeed();
    
    	int iPosX, iPosY;
    
    	std::chrono::steady_clock::time_point lastMove;
    
    	bool operator==(const CFallingChar& rhs);
    
    private:
            char cChar = 'X';
    
    	int iSpeed;
    };
    

    CPP-File:

    #include "FallingChar.h"
    #include "ScreenManager.h"
    
    CFallingChar::CFallingChar()
    {
    	iSpeed = 1 + rand() % 3;
    
    	lastMove = std::chrono::steady_clock::now();
    	iPosX = rand() % CScreenManager::GetInstance()->iScreenWidth;
    	iPosY = CScreenManager::GetInstance()->iScreenHeight;
    }
    
    int CFallingChar::GetSpeed()
    {
    	return iSpeed;
    }
    
    bool CFallingChar::operator==(const CFallingChar& lhs)
    {
    	return false; //versuche hier zu debuggen, darum nur das return false
    }
    

    Mein Problem ist, dass ich diese Klasse in einem std::vector nutze und im Hintergrund beim durchiterieren durch diesen Vector versucht wird, mit einem == operator zwei Objekte dieser Klasse miteinander zu vergleichen.
    Visual Studio sagt
    "C2678 Binärer Operator "==": Es konnte kein Operator gefunden werden, der einen linksseitigen Operanden vom Typ "CFallingChar" akzeptiert (oder keine geeignete Konvertierung möglich)"

    Ich habe auf verschiedene Arten versucht den Operatoren zu überladen (auch mit lhs und rhs als Parameter), jedoch ohne Erfolg, der Fehler ist immer der gleiche. Irgendwas mache ich dann offensichtlich falsch. Habt ihr ne Idee?



  • Dir fehlt ein const => bool operator==(const CFallingChar& rhs) const
    Zu erwähnen wäre allerdings noch, dass man das gerne auch als hidden friend definiert:
    friend bool operator==(const CFallingChar& lhs, const CFallingChar& rhs)
    Ob man die friend Variante oder die Member Funktion bevorzugt, ist mehr oder weniger Geschmacksache. Kommt auch ein wenig darauf an, in welchem Kontext das Ganze stattfindet. Ich persönlich nutze lieber den hidden friend.

    Seit c++20 kann man in diversen Fällen, diesen Operator auch einfach defaulten:
    friend bool operator==(const CFallingChar& lhs, const CFallingChar& rhs) = default;

    Das ist dann erlaubt, wenn alle Member der Klasse mittels operator == vergleichbar sind.



  • @DNKpp Deklariere ich im Header

    bool operator==(const CFallingChar& rhs) const;
    

    und definiere im CPP File

    bool CFallingChar::operator==(const CFallingChar& rhs) const
    {
    	return false;
    }
    

    bekomme ich den gleichen Fehler.

    Übrigens sieht der Code der den Fehler erzeugt so aus:

        for (; _First != _Last; ++_First) {
            if (*_First == _Val) {                       //hier operator==
                break;
            }
        }
    


  • Ich kann jetzt auf den ersten Blick nicht erkennen, was falsch sein könnte. Das liegt aber auch daran, dass du zu wenig Kontext lieferst. Dein Code Schnipsel sieht mir stark nach irgendeinem stl algorithmus, wahrscheinlich std::find aus. Am besten du zeigst mal alles relevante (incl. der Fehlermeldung), oder packst es direkt auf godbolt.org. Dann kann man sich auch besser ein Bild davon machen.



  • @DNKpp
    Hier der Code mit dem std::vector:

    #include "ScreenManager.h"
    #include "Player.h"
    #include "GameManager.h"
    #include <Windows.h>
    #include <thread>
    #include <chrono>
    
    CScreenManager::CScreenManager()
    {
        HANDLE hstdin;
        CONSOLE_SCREEN_BUFFER_INFO csbi;
    
        hstdin = GetStdHandle(STD_OUTPUT_HANDLE);
    
        GetConsoleScreenBufferInfo(hstdin, &csbi);
    
        iScreenWidth = csbi.srWindow.Right - csbi.srWindow.Left + 1;
        iScreenHeight = csbi.srWindow.Bottom - csbi.srWindow.Top + 1;
    
        fallingChars = std::vector<CFallingChar>();
        CFallingChar* firstChar = new CFallingChar();
    
        fallingChars.push_back(*firstChar);
    }
    
    void CScreenManager::StartRenderLoop()
    {
        std::thread threadobj(&CScreenManager::DoRenderTicks);
    }
    
    void CScreenManager::DoRenderTicks()
    {
        HANDLE hstdin;
    
        hstdin = GetStdHandle(STD_OUTPUT_HANDLE);
    
        while (true)
        {
            if (fallingChars.size() <= 0) return;
    
            for (size_t i = 0; i < fallingChars.size(); i++)
            {
                std::chrono::steady_clock::time_point now;
                now = std::chrono::steady_clock::now();
    
                int moveTime = 1000 / fallingChars[i].GetSpeed();
                CFallingChar *current = &fallingChars[i];
    
                if (std::chrono::duration_cast<std::chrono::milliseconds>(now - current->lastMove).count() > moveTime)
                {
                    current->lastMove = now;
                    current->iPosX += 1;
    
                    if (current->iPosX == CPlayer::GetInstance()->GetXPos() - 1)
                    {
                        CGameManager::GetInstance()->FallingCharCatch(current);
                    }
                    else if (current->iPosX >= iScreenHeight)
                    {
                        std::vector<CFallingChar>::iterator it = std::find(fallingChars.begin(), fallingChars.end(), current);
    
                        if (it != fallingChars.end())
                        {
                            fallingChars.erase(it);
                        }
                    }
                }
            }
        }
    }
    
    void CScreenManager::AddFallingChar(CFallingChar& pFallingChar)
    {
        fallingChars.push_back(pFallingChar);
    }
    
    

    ScreenManager.h:

    #pragma once
    #include <vector>
    #include "FallingChar.h"
    class CScreenManager
    {
    public:
    
    	static CScreenManager* GetInstance()
    	{
    		if (instancePtr == nullptr || instancePtr == 0)
    		{
    			instancePtr = new CScreenManager();
    		}
    		return instancePtr;
    	}
    
    	CScreenManager();
    
    	int iScreenWidth, iScreenHeight;
    
    	void StartRenderLoop();
    	void AddFallingChar(CFallingChar& FallingChar);
    
    private:
    	std::vector<CFallingChar> fallingChars;
    
    	void DoRenderTicks();
    
    	static CScreenManager* instancePtr;
    };
    
    
    

    FallingChar.cpp:

    #include "FallingChar.h"
    #include "ScreenManager.h"
    
    CFallingChar::CFallingChar()
    {
    	iSpeed = 1 + rand() % 3;
    
    	lastMove = std::chrono::steady_clock::now();
    	iPosX = rand() % CScreenManager::GetInstance()->iScreenWidth;
    	iPosY = CScreenManager::GetInstance()->iScreenHeight;
    }
    
    int CFallingChar::GetSpeed()
    {
    	return iSpeed;
    }
    
    bool CFallingChar::operator==(const CFallingChar& rhs) const
    {
    	return false;
    }
    
    

    und FallingChar.h:

    #pragma once
    #include <chrono>
    class CFallingChar
    {
    public:
    	CFallingChar();
    
    	int GetSpeed();
    
    	int iPosX, iPosY;
    
    	std::chrono::steady_clock::time_point lastMove;
    
    	bool operator==(const CFallingChar& rhs) const;
    
    private:
        char cChar = 'X';
    
    	int iSpeed;
    };
    
    
    

    Screenshot der Fehlermeldung:

    https://imgur.com/a/6vVJRsq

    Die Sprachversion ist C++14



  • Naja, dein current in Zeile 60 ist ein Pointer (siehe Zeile 47 CFallingChar *current); den musst du natürlich dereferenzieren => std::vector<CFallingChar>::iterator it = std::find(fallingChars.begin(), fallingChars.end(), *current);

    Allgemein würde ich dir für den return type auto ans Herz legen. Solche komplizierten return types manuell zu schreiben, hat man eigentlich zu c++11 aufgehört =>
    auto it = std::find(fallingChars.begin(), fallingChars.end(), *current);

    Und wenn du mit c++20 arbeitest kannst du mit den ranges algorithmen sogar noch ein bisschen was einsparen =>
    auto it = std::ranges::find(fallingChars, *current);

    Immerhin schön zu sehen, dass du mit c++ algorithmen (std::find) arbeitest und das nicht selbst implementierst (machen leider viel zu viele). 🙂
    Dennoch: Dein Code ist ein wenig zu umständlich. Du kennst bereits die Position, an der sich current befindet (nämlich i). Damit kannst du dir das Suchen sparen und direkt löschen:
    fallingChars.erase(fallingChars.begin() + i);



  • Der Code von @Steff00212 (also das Suchen nach einem Wert, obwohl man den Index schon kennt), erinnert damit (ein wenig) an "Shlemiel the painter’s algorithm" bekannt durch Joel on Software: Back to Basics.



  • Wie lautet das bekannte Sprichtwort: Manchmal sieht man den Wald vor lauter Bäumen nicht. Ich habe mich so auf den operatoren fixiert, dass ich an garkeiner anderen Stelle mehr nach Problemen gesucht habe 😃

    Und zu @Th69: Oder auch eine Rube-Goldberg-Maschine, wobei ich ja schon irgendwas mache 😃

    Ich weiß nicht, warum ich nicht daran gedacht habe, dass ich den Index schon kenne. Hab das geschrieben ohne über den vorherigen Code nachzudenken. Dabei bin ich kein neuer Programmierer, ich schwöre! 😃


Anmelden zum Antworten