Avatar
Adam Gajdečka:4. dubna 23:22

Napsal jsem jednoduchou knihovnu pro napojení na API mautic.org

https://github.com/…sakCZ/Mautic

Prosím o rady, co a jak vylepšit. Dokumentace API Mautic je zde http://developer.mautic.org/

Děkuji

 
Odpovědět 4. dubna 23:22
Avatar
Petr Čech
Redaktor
Avatar
Odpovídá na Adam Gajdečka
Petr Čech:4. dubna 23:41

Uff, to je hrůza, až mě bolí oči. Proč proboha tolik statiky? Nenarazil jsem na jediný kus kódu, kde by měla opodstatnění. Jestli chceš argumentovat tím, že od toho budeš mít vždy jen jednu instanci, tak od toho je singleton, který se začíná čím dál víc chápat jako antipattern - např. Settings - co když to budeš chtít třeba ukládat do souboru? Budeš mít smůlu a celé to budeš muset přepsat.
Ve zkratce - to, že od něčeho bude jen jedna instance není důvod pro statiku, ta je k něčemu jinému - např. extension metody, nějaké primitivní helper metody a pod. Jelikož očividně začínáš, doporučil bych statiku nepoužívat, nikdy není třeba. Časem bys měl přijít na to, co má smysl udělat statické - nejlepší je asi odkoukat to od jazyka (resp. jeho standardní knihovny)
Všimni si třeba, že var client = new RestClient(Settings.UrlApi); voláš strašně často, to dost smrdí špatným návrhem a duplikací kódu, ne? ;) A to nehledě na to, že to asi nebude nijak moc efektivní, jestli si třeba každá nová instance musí otevírat nový socket...

Models/Contac­tModel.cs:
Ale fuj, třídy se píší až na extrémní výjimky každá do svého souboru (opět, prostě je vždy dávej do svého souboru).

namespace namespace MauticApiLibrary.Models.GetContacts - namespace typicky odpovídá adresářové struktuře a je dobrý nápad nechat se o to starat IDE - VisualStudio je zrovna v tomto perfektní, když si otevřeš properties složky, je tam nastavení toho, jak se to má generovat (a udržovat!)

Proč máš naimportovaný namespace System.Threadin­g.Tasks? Pokud začínáš, pokusy o paralelní zpracování bych opravdu nechal být, je to docela vysoká magie.

TL;DR celé bych to smazal a napsal znovu s tím, že se budu snažit to napsat pořádně.

Nahoru Odpovědět 4. dubna 23:41
the cake is a lie
Avatar
Adam Gajdečka:4. dubna 23:49

Jak používat singleton místo statiky mi není jasné

  • ok, dává to smysl to hodit do více souborů
  • namespace jsem zapomněl nějak upravit, přehazoval jsem to z jiné složky, měnil trochu strukturu
  • System.Threadin­g.Tasks ten se mi tam sám přidal

A máš prosím nějaký tip, kde si můžu prohlédnout jednoduchou API knihovnu a brát ji jako vzor?

 
Nahoru Odpovědět 4. dubna 23:49
Avatar
Odpovídá na Petr Čech
Adam Gajdečka:4. dubna 23:59

Budu muset po odstranění statiky vytvářet například vždy Contacts contacts = new Contacts(); ?

 
Nahoru Odpovědět 4. dubna 23:59
Avatar
Petr Čech
Redaktor
Avatar
Petr Čech:5. dubna 0:49

Singleton nepoužívat, je to vlastně pořád statika. Očividně vůbec nechápeš principy OOP, zahoď všechno, co děláš a nauč se to. Tady je dost fajn kurz.
Takto v žádném případě nemůžeš pokračovat, věř mi. A nebo mi nevěř a zjistíš to "the hard way", až budeš muset něco úplně celé přepsat :D

Nahoru Odpovědět 5. dubna 0:49
the cake is a lie
Avatar
Petr Čech
Redaktor
Avatar
Petr Čech:5. dubna 0:54

Zajímavá třída na inspiraci by mohlo být třeba DateTime, nic lepšího mě teď napadlo. Hledej .NET API browser, někde se tam dá proklikat ke zdrojakům.
Jen bych chtěl upozornit na to, že není dobré se inspirovat třídou Console, ta je velice specifická.

Nahoru Odpovědět 5. dubna 0:54
the cake is a lie
Avatar
Petr Čech
Redaktor
Avatar
Petr Čech:5. dubna 13:27

Abych byl upřímný, já bych se sám za SW inženýra dost dobrého na to, aby mohl ukazovat ostatním cestu neoznačil, jen jsem vypíchl to, co je naprosto objektivně špatně. Možná Marian Benčat by uměl říct, co je správná cesta.

Nahoru Odpovědět  +1 5. dubna 13:27
the cake is a lie
Avatar
Tomas
Člen
Avatar
Tomas:5. dubna 17:52

Singleton nepoužívat, je to vlastně pořád statika.

Chtěl bych se tedy zeptat, jak chceš řešit situace, kdy v programu má smysl mít pouze jedno instanci dané třídy.
Díky

 
Nahoru Odpovědět 5. dubna 17:52
Avatar
Odpovídá na Štefan Melich
Adam Gajdečka:5. dubna 23:33

Díky. Určitě si to celé nastuduji

 
Nahoru Odpovědět 5. dubna 23:33
Děláme co je v našich silách, aby byly zdejší diskuze co nejkvalitnější. Proto do nich také mohou přispívat pouze registrovaní členové. Pro zapojení do diskuze se přihlas. Pokud ještě nemáš účet, zaregistruj se, je to zdarma.

Zobrazeno 10 zpráv z 10.