Fabryka obiektów IShape jako singleton – poszukiwanie źródła błędu

0

Cześć,
Jak w temacie, muszę stworzyć fabrykę obiektów IShape, która jest singletonem, dodatkowo ma ona mieć możliwość sprawdzenia czy jakiś obiekt jest tworem tej fabryki, niestety utknąłem, moja fabryka wygląda tak:

#pragma once
#include "IShapeFactory.h"
#include "Circle.h"
#include "Triangle.h"
#include "Rectangle.h"
#include <list>


class IShapeFactory
{
	list<IShape*> creationList;

private:
	IShapeFactory();
	IShapeFactory(const IShapeFactory &);
	IShapeFactory& operator=(const IShapeFactory&);
	virtual ~IShapeFactory();
public:
	static IShapeFactory& getInstance()
	{
		static IShapeFactory* instance = NULL;
		if (!instance)
		{
			static IShapeFactory instance = new IShapeFactory();
		}
		return *instance;
	}

	IShape* CreateShapeInstance(ShapeEnum a_eShape)
	{
		if (a_eShape == circle) {
			Circle* wskshape = 0;
			Circle shape;https://github.com/FranekWKimono/SingletonFactory
			wskshape = &shape;
			creationList.push_back(wskshape);
			IShape* iShape = wskshape;
			return iShape;
		}
		else if (a_eShape == triangle)
		{
			Triangle* wskshape = 0;
			Triangle shape;
			wskshape = &shape;
			creationList.push_back(wskshape);
			IShape* iShape = wskshape;
			return iShape;
		}
		else if (a_eShape == rectangle)
		{
			Rectangle* wskshape = 0;
			Rectangle shape;
			wskshape = &shape;
			creationList.push_back(wskshape);
			IShape* iShape = wskshape;
			return iShape;
		}
		else {
			cout << "nie ma takiej figury" << endl;
			return 0;
		}
	}

	bool IsFactoryChildren(IShape* a_oShapeObj) {
		bool found = (std::find(creationList.begin() , creationList.end(), a_oShapeObj) != creationList.end());
		return found;
	}

	void Destroy() {
		creationList.clear();
	}

};


Funkcja main:

IShapeFactory::getInstance().CreateShapeInstance(triangle);

co w połączeniu daje mi błędy typu:

Error    LNK2019    unresolved external symbol "public: static class IShapeFactory __cdecl IShapeFactory::getInstance(void)" (?getInstance@IShapeFactory@@SA?AV1@XZ) referenced in function _main 

Od czego w ogóle powinienem zacząć poszukiwania tego błędu?

0

Przenieś implementację IShapeFactory::getInstance do cpp

1

Już Ci mogę powiedzieć, że będziesz miał problemy z pamięcią. Przede wszystkim tworzenie singletonu. To powinno wyglądać inaczej. Prościej:

static ShapeFactory & getInstance()
{
  static ShapeFactory factory;
  return factory;
}

I tyle jeśli chodzi o singleton. W Twoim przypadku zostajesz ze wskaźnikiem, który nigdy nie będzie zwolniony. W moim przykładzie obiekt factory zawsze zostanie zwolniony na koniec aplikacji.

Poza tym nie posługuje się gołymi wskaźnikami. To nie lata 90te. Masz smart pointery (np. std::shared_ptr). Korzystaj z nich.

Kolejna rzecz. Drabinka: if..else. Zrób z tego normalną konstrukcję: switch..case. To powinno wyglądać tak:

IShape * pShape = nullptr;

switch(a_eShape)
{
  case circle:
    pShape = new circle();
    break;

  case triangle:
    pShape = new triangle();
    break;
//itd.
}

creationList.push_back(pShape);
return pShape;

Oczywiście zamiast gołych wskaźników - shared_ptr.

Co więcej, nie potrzebujesz tutaj listy. Możesz posłużyć się unordered_set.

2

Żeby było jasne , skąd moja rada.
Ten błąd:

Error LNK2019 unresolved external symbol "public: static class IShapeFactory __cdecl IShapeFactory::getInstance(void)" (?getInstance@IShapeFactory@@SA?AV1@XZ) referenced in function _main

to błąd linkowania. Nie wskazuje on problemu ze składnią, ale z organizacją komponentów projektu.
Czemu linker narzeka, że nie może odnaleźć IShapeFactory::getInstance?
Moim zdaniem masz więcej błędów, a to jest skutek uboczny tych błędów.
Zmienna static IShapeFactory* instance = NULL; znajdująca się wewnątrz IShapeFactory::getInstance ma globalny czas życia i musi byś związana z konkretną jednostką translacyjną (źródłem cpp).
Jako, że umieściłeś IShapeFactory::getInstance w nagłówku kompilator próbuje (może ale nie musi) zrobić z niego funkcję inline i nie wie gdzie (w jakim cpp) ma umieścić tą zmienną.
Jeśli zadeklarujesz tą funkcję i zdefinijuesz ją w jednym cpp, kompilator umieści zmienną w konkretnym cpp i wygeneruje prawidłową funkcję, dzięki czemu linker powinien być zadowolony.

PS.

  1. Singleton to jest anty pattern
  2. Twój kod jest niepoprawny (masz dwie zmienne static)
  3. Wymaganie wiedzy, kto wyprodukował obiekt jest absurdalne, szczególnie jeśli używasz singleton-a.
  4. Fabryka realizowana tylko w nagłówku jest bezsensu, bo nie wprowadza izolacji między implementacjami interfejsu IShape a użytkownikami fabryki.
  5. Prefiks I oznacza, że klasa jest interfejsem, czyli klasą posiadającą jedynie publiczne metody czysto wirtualne i nic więcej (za wyjątkiem destruktora). IShapeFactory nie spełnia tego warunku.
1
MarekR22 napisał(a):
  1. Singleton to jest anty pattern

Ale ma swoje zastosowania i się go używa. Stał się antipattern, bo ludzie stosowali go wszędzie. I nie dość, że nieprawidłowo, to jeszcze w sytuacjach, do których nie został wymyślony. Jeśli nie ma innego sposobu (np. kontenery IOC), to nie ma nic złego w zrobieniu z fabryki singletonu.

0

Faktycznie trochę musiałem pozmieniać, dopiero zaczynam z C++ i cała ta struktura z nagłówkami początkowo była dla mnie niejasna. Dziękuję wszystkim za pomoc.

1 użytkowników online, w tym zalogowanych: 0, gości: 1