Dotaz: OOP návrh aplikace

[Beda0]

Zdravím, dělám aplikaci v Qt, která načítá data a zobrazuje v grafické podobě. Data mohou pocházet z různých zdrojů - xml soubory, modbus po TCP a v budou přibývat další. Jaký konkrétní typ komunikace se použije závisí na nastavení programu. Pro načítání dat mam interface

    virtual int read(struktura1 *data)=0;
    virtual int write(struktura1 *data)=0;
    virtual int read(struktura2 *data)=0;
    virtual int write(struktura2 *data)=0;

Tento interface pak je konkrétně implementován pro xml a modbus.

Dále mám třídu readWriteFactory, která má metodu

ReadWrite* ReadWriteFactory::chooseRWType(int type)
{
    ReadWrite *ret;
    switch(type)
    {
    case 1:
        ret = new ReadWriteModbus();
        return ret;
        break;
    default :
        ret = new ReadWriteXML();
        return ret;
        break;
    }
}

tímhle si vytvořím instanci konkrétní třídy pro komunikaci, která splňuje interface a pak v programu neřeším, jakým způsobem data získávám. Instanci třídy si takhle vytvářim všude kde jí potřebuju - v každym widgetu, kterj potřebuje načítat nebo ukládat data. Dál pak používám jen rw->read(&data) kde data je struktura, kterou chci naplnit daty.

Modbus na rozdíl od xml souborů potřebuje navázat spojení a udržovat někde identifikátor toho spojení. To je řešeno tím, že po spuštění aplikace se zjistí, jaký je nastavený typ spojení, pokud je to modbus, tak se zavolá funkce, která to spojí a identifikátor se uloží do globální proměnné. Pokaždký když pak volám rw->read pro modbus, tak se identifikátor spojení načte z globální proměnné. Vím, že to je navrženo špatně a chci to zlepšit. Dlouho jsem to chtěl předělat, ale dělal sem věci, které byly někde vidět a na tohle se nedostalo.

Teď potřebuju aplikaci rozdělit do knihoven - datalogic a businesslogic budou v čistém C++ jako knihovny a v aplikaci zbude GUI v Qt. Problém je s používáním globálních proměnných.

Už si s tím lámu docela dlouho hlavu, ale moc mi to nejde. Vymyslel jsem, že interface změním na

    virtual int read(struktura1 *data,connectionClass identifikator)=0;
    virtual int write(struktura1 *data,connectionClass identifikator)=0;
    virtual int read(struktura2 *data,connectionClass identifikator)=0;
    virtual int write(struktura2 *data,connectionClass identifikator)=0;

Kde třída identifikator bude uchovávat informace o typu připojení a identifikátor spojení, pokud existuje. Mám představu, že vytvořím instanci třídy connectionClass na začátku programu nebo při manuálním spojení a tu pak budu předávat rw->read(&data,id).

A teď jedna z věcí, která mi v OOP pořád nejde na rozum - jak mam předávat id? rw->read bude volaná někde úplně z nejnižšího QtWidgetu, kterej nad sebou má další 3 parent widgety - jak tam to id dostanu? Nejjednoduší by bylo udělat id nějakym způsobem globální, ale všude píšou, že to je špatně, že to neni thread safe a další věci.

Je takhle navržená struktura správná? Jak mam správně předávat id?

Doufám, že to je popsaný srozumitelně a nic sem nevynechal. Díky.

---

---

Odpovědi

[Delaunay]

Redundantní předávání identifikátoru jako argument read() nebo write() je samozřejmě veskrze špatným řešením. Nejen z důvodu neefektivnosti, ale především z pohledu návrhu.

C++ jsem už bohužel hodně let neviděl, ale nejspíše bych to přepsal takto (záměrně vypuštěna členská funkce write()):

Struktury:

typedef struct Data {} Data;
typedef struct Data2 {} Data2;

Třída pro zapouzdření případných kontextových dat:

class Context
{
    private:
        int sourceType;
        ModbusConnection* modbusConn;

    public:
        Context() : sourceType(0), modbusConn(0) {}

        int getSourceType();
        void setSourceType(int type);

        ModbusConnection* getModbusConnection();
        void setModbusConnection(ModbusConnection* conn);
};

Připojení k modbus:

class ModbusConnection
{
    public:
        ~ModbusConnection();
};

Rozhraní pro čtení:

class Reader
{
    public:
        virtual ~Reader() {}
        virtual int read(Data& data) = 0;
        virtual int read(Data2& data) = 0;
};

Třídy implementující čtení:

class ModbusReader : public Reader
{
    private:
        ModbusConnection* conn;

    public:
        ModbusReader(ModbusConnection* connection) : conn(connection) {}
        int read(Data& data);
        int read(Data2& data);
};

class XMLReader : public Reader
{
    public:
        int read(Data& data);
        int read(Data2& data);
};

Factory třída:

Reader* ReaderFactory::get(Context* context)
{
    Reader *ret = 0;
    int type = context->getSourceType();
    switch(type)
    {
        case 1:
            ret = new ModbusReader(context->getModbusConnection());
            break;
        default :
            ret = new XMLReader();
    }
    return ret;
}

A konečně pak to vše nějak zavoláme:

ModbusConnection modbus;

Context context;
context.setSourceType(1);
context.setModbusConnection(&modbus);

Reader *reader = ReaderFactory::get(&context);
Data data;
while (reader->read(data))
{
    // .... 
}
delete(reader);

[Beda0]

Díky, takhle nějak sem si to představoval, ale pár věcí mi pořád unikalo a dělal sem je špatně.

Ale pořád mi uniká jedna zásadní věc - já potřebuju reader->read(data) volat na mnoha různejch místech - v několika objektech úplně jinde, než otvírám spojení a nevim, jak předávat instanci Reader.

class MainClass
{
public: MainClass();
        ~MainClass();
}

MainClass()
{
odbusConnection modbus;

   Context context;
   context.setSourceType(1);
   context.setModbusConnection(&modbus);

   Reader *reader = ReaderFactory::get(&context);
   obj = new Class1();
}

~MainClass()
{
   delete(reader);
}

Class1()
{
   obj = new Class2();
   obj = new Class3();
}

Class2()
{
   Data data;
   reader->read(data);
}

Jediný, co mě napadá, je prostě to předávat v konstruktoru nějak takhle:

class Class1
{
   public Class1(Reader *reader);
}
class Class2
{
   public Class2(Reader *reader);
}
Class1(Reader *reader)
{
   obj = new Class2(reader);
   obj = new Class3(reader);
}
Class2(Reader *reader)
{
   Data data;
   reader->read(data);
}

Ale nevím, jestli to je takhle správně a nelíbí s mi to, protože musím udělat vlastní třídu Class1 a nemůžu použít standardní (jsou to widgety GUI).

[Beda0]

Ta poslední věta je nesmysl - stejně ty třídy dělám sám. Takže neni problém to takhle udělat, jen nevim, jestli je to takhle správně.

[Josef Kufner]

Ty nepotřebuješ volat reader->data všude možně. Ty to potřebuješ volat jen v jedné třídě, která patří v MVC pod písmenko M (model).

Ty widgety nemají co hrabat na nějaký reader někde. Nech je jen přeposlat signály nadřazenýmu widgetu a tam dělej, co potřebuješ.

Dependency injection přesně tohle řeší, uniká ti však ještě jedna vrstva abstrakce.

[Beda0]

Jo, to bude asi ono, snad mi to došlo... Díky moc za navedení správnym směrem.

[XY]

Nieco podobne som riesil nedavno a dosiel som k velmi zaujimavej serii clankov:

http://codebetter.com/jeremymiller/2007/07/26/the-build-your-own-cab-series-table-of-contents/

Je to sice na dlhsie citanie, ale odporucam.

[Josef Kufner]

Proč máš break za returnem?

Instance třídy, která komunikuje se nevyrábí znovu pokaždé, když ji potřebuješ, ale získáš jí od někud jinud. Veškeré otvírání spojení se udělá v konstruktoru a potřebné informace o spojení si uložíš do private proměnných v té instanci.

Pro komunikaci pak používáš pořád ten jeden objekt. Žádný widget by neměl volat new pro vytvoření spojení, k tomu máme techniky známé pod označením "Dependency injection" (viz seriál na root.cz). Pokud je načítání omezené jen v rámci jednoho dialogu s progresbarem ći podobné jednorázové akce, použij Factory, tedy specializovanou třídu, která ti vytvoří instanci readeru dle zadaných parametrů. Tahle factory se ti bude hodit v každém případě.

Ta factory by měla být napsána nějak obecně, aby při vytovoření nového rozhraní nebylo třeba do ní nic dopisovat. Během kompilace by bylo vhodné automaticky vygenerovat (jednoduchým skriptem) hlavičkový soubor se seznamem všech známých readerů (prosté pole struktur), které by pak ta factory procházela a podle něj vyráběla instance tříd.

[Josef Kufner]

Jo a až si budeš tak číst o návrhových vzorech a narazíš na Singleton, tak ho přeskoč. Sice asi budeš mít pocit, že to je přesně to co potřebuješ, ale tento pocit je jen zákeřná iluze. A pokud jejímu pokušení podlehneš, budeš krutě litovat.

[Šangala]

Proč máš break za returnem?

Možná proto aby uspokojil některé validátory - už jsem je tam kdysi dopisoval, bo mě štvaly warning-y.

[Josef Kufner]

Pokud je validátor tak hloupý, že nechápe return místo break, tak bych mu moc nevěřil.

[Šangala]

Je to určitě hloupost validátoru, nicméně když je to integrovaný a jinak plní svou službu dobře, tak cesta nejmenšího odporu je ty break dopsat (stejně jako třeba zbytečný zápis virtual ~InterfaceAsPureAbstractClass(){/* nothing to do */};).
Dělalo mě to buď ve Fazolích nebo Zatmění, ale už si nepamatuji jazyk ale asi C++ nebo PHP. Ale každopádně teď (rozuměj NOW()- 2 years) v Zatmění s C++ to nedělá…

[Beda0]

Break sem tam zapomněl, předtim to vypadalo jinak a return byl až na konci.

Díky za odkaz na seriál, přečetl jsem to, ale jestli sem to pochopil správně, tak to neřeší můj hlavní problém viz komentář výše.

[biolog]

Jak píše Josef, vytvářet pořád nové instance komunikace je nepraktické. Není to užitečné nyní ani později a jiným, složitější protokolům by to mohlo vadit. Předpokládám že "widget" znamená "Qt grafický prvek".

Moje řešení:

Když se spouští tvá aplikace, na základě command-lajny nebo konfiguráku vytvoříš ModbusReader nebo XMLReader. V případě ModbusReader jí při vytváření předáš číslo TCP portu, nebo rovnou TCP stream, nebo co se ti líbí. Pokud budeš vytvářet ModbusReader nebo XMLReader z více míst (asi ne), nebo ten kód na rozhodování se mezi nimi a jejich vytváření bude delší než cca 10 řádek (asi ne), tak ten kód dáš do samostatné funkce. Můžeš jí říkat factory, ale stejně nebude mít žádný stav.

Při spouštení aplikace při vytváření těch widgetů jim předáš ten ModbusReader nebo XMLReader.

Pokud chceš, tak ten ModbusReader nebo XMLReader ulož do globální proměnné, ale není to potřeba, a hlavně widgety používají ten svůj správný (sdílený) objekt, a tedy nevýhoda globálních proměnných tady nehraje roli.

Pokud by bylo potřeba k ModbusReader a XMLReader přidat dalšího potomka rozhraní ReadWrite, tak upravíš ten kód na vytváření (ať už je v samostané "factory" funkci nebo ne). Pokud bude potřeba mít více XML nebo Modbus zdrojů (se svými vlastními widgety), tak jen upravíš ten kód, který ví o command-lajně a konfiguráku, proste tam kudy jde ta informace o connectování.

Je to tak snadné, že psát sem kód by bylo trapné.

[Beda0]

Tak jsem to vyřešil, asi nejjednodušší bude malá ukázka kódu:

Class Trida : public QMainWindow{
Q_OBJECT

public:
    QWidget *novyWidget;
public slots:
    void readData(Struktura* data);
    void //tady budou sloty pro cteni a zapis vsech struktur a dalsi veci - connect, disconnect atd.
private:
    ReadWrite *readWrite;
}



Trida::Trida(QWidget *parent) : QMainWindow(parent){
    cnt = new Context();
    cnt->set....
    //tady naplnim obsah cnt z konfiguracniho souboru
    //cnt uchovava informace pro vsechny spojeni - ip, port, modbus_identifikator_spojeni,
    //umisteni_xml_souboru, jestli je to nekam aktkualne pripojeno a dalsi...

    ReadWriteFactory *readWriteFactory = new ReadWriteFactory();
    readWrite = readWriteFactory->sellect(cnt);//tohle mi vrati jeden konkretni typ spojeni
    readWrite->setContext(cnt);
    readWrite->connect(); //pripojim se - pro typy pripojeni, ktere nevyzaduji spojeni to neni potreba,
                   // ale nicemu to nevadi

    novyWidget = new NovyWidget();
    connect(novyWidget,SIGNAL(readData(Struktura*)),this,SLOT(readData(Struktura*)));
    //pomoci signalu a slotu spojim pozadavaky na data se slotem, kterej je tady
    //budu pridavat vic slotu - na cteni, zapis, disconnect, novej connect atd...
}

Trida::~Trida()
{
    readWrite->disconnect();
    delete ui;
}
void Trida::readData(Struktura* data)
{
    readWrite->read(data);
}

Vlastni cteni dat se pak dela vyslanim signalu
Struktura *data;
data = new Struktura;
emit readData(data);

Všem díky moc za pomoc.

[Josef Kufner]

Základní myšlenkou Dependency Injection je oddělení závislostí a delegace starosti o ně mimo závisející objekty.

Použití factory je fajn, ale pořád závisíš na té factory. Takže to prostě vykopej ven celé.

• Slot a signál je v pohodě, asi bych jen přijímající slot pojmenoval vhodněji.
• Z kostruktoru Tridy vykopej použití factory a ponech tam jen propojení vnořeného widgetu.
• Ve funkci main, když vyrábíš okno, vyrob i Factory a tu předej tomu oknu (jako parametr konstruktoru).

[Beda0]

"Takže to prostě vykopej ven celé" Co celé?

To mam do main přesunout jen vytváření factory a jako parametr konstruktoru předat instanci factory? Nebo tam ma přesunout všechno - i naplnění context a vytvoření instance ReadWrite a pak předávat konstruktoru instanci ReadWrite?

To první chápu smysl, ale je to jen jeden řádek

ReadWriteFactory *readWriteFactory = new ReadWriteFactory();

To druhý nevim jak, protože v potřebuju za běhu aplikace měnit typy připojení = mazat a vytvářet nový instance ReadWrite...

[Josef Kufner]

To mam do main přesunout jen vytváření factory a jako parametr konstruktoru předat instanci factory?

Ano.

Nebo tam ma přesunout všechno - i naplnění context a vytvoření instance ReadWrite

To je práce pro tu factory.

To druhý nevim jak, protože v potřebuju za běhu aplikace měnit typy připojení = mazat a vytvářet nový instance ReadWrite...

Takže si budeš hrát s factory, kterou dostaneš zvenčí (z main). Ono se mi tam trošku namíchaly dvě myšlenky, kdy první pocházela z doby než jsem si uvědomil, že asi nebudeš chtít pracovat jen nad jedním spojením.

[Beda0]

Jo, takhle už mi to je jasný... díky moc za pomoc, osvětlil si mi spoustu věcí. Evidentně sem v tom měl daleko větší zmatek, než sem si připouštěl na začátku.