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:
Die Sprachversion ist C++14
-
Naja, dein
current
in Zeile 60 ist ein Pointer (siehe Zeile 47CFallingChar *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 sichcurrent
befindet (nämlichi
). 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!