Galerie programátorských hříchů (díl 1.)

Tomáš Herceg       4. 3. 2009       C#, VB.NET       7087 zobrazení

Posledních pár dní si hraju s aplikací BlogEngine.NET, což je open source blogovací engine napsaný v ASP.NET. Pro pořádné otestování jsem ji nasadil na svůj osobní web, jako obsah posloužil tento blog. Při rozšiřování a “dobastlování” aplikace této jsem narazil na pár neduhů, které bohužel vídám na můj vkus až příliš často i v jiných produktech. Možná jsem až moc velký puntičkář co se týče úpravy a kvality kódu, ale v některých konstrukcích vidím potencionální problémy do budoucna, a proto jsem se rozhodl založit tento seriál s programátorskými hříchy.

Následující kousek kódu jsem si vypůjčil právě z BlogEngine.NET, abych na něm demonstroval první hřích. Jednoduše bere stringy z Resources (to se dělá uvnitř metody Translate, ta nás ale až tak nezajímá, používá se na mnoha místech projektu) a vygeneruje z nich javascript do stránky, protože tyto řetězce musí být v lokalizované podobě dostupné v klientských skriptech. Protože řetězce se uzavírají do apostrofů, je jasné, že pokud by string apostrof sám obsahoval, nastal by nám problém. Proto je tedy třeba apostrofy zaescapovat.

        /// <summary>
        /// Adds the localization keys to JavaScript for use globally.
        /// </summary>
        protected virtual void AddLocalizationKeys()
        {
            StringBuilder sb = new StringBuilder();
            sb.AppendFormat("KEYhasRated='{0}';", Translate("youAlreadyRated").Replace("'", "\\'"));
            sb.AppendFormat("KEYratingOK='{0}';", Translate("ratingWasRegistered").Replace("'", "\\'"));
            sb.AppendFormat("KEYwebRoot='{0}';", Utils.RelativeWebRoot);
            sb.AppendFormat("KEYsavingTheComment='{0}';", Translate("savingTheComment").Replace("'", "\\'"));
            sb.AppendFormat("KEYcomments='{0}';", Translate("comments").Replace("'", "\\'"));
            sb.AppendFormat("KEYcommentWasSaved='{0}';", Translate("commentWasSaved").Replace("'", "\\'"));
            sb.AppendFormat("KEYcommentWaitingModeration='{0}';", Translate("commentWaitingModeration").Replace("'", "\\'"));
            sb.AppendFormat("KEYcancel='{0}';", Translate("cancel").Replace("'", "\\'"));
            sb.AppendFormat("KEYfilter='{0}';", Translate("filter").Replace("'", "\\'"));
            sb.AppendFormat("KEYapmlDescription='{0}';", Translate("filterByApmlDescription").Replace("'", "\\'"));

            HtmlGenericControl script = new HtmlGenericControl("script");
            script.Attributes.Add("type", "text/javascript");
            script.InnerHtml = sb.ToString();
            Page.Header.Controls.Add(script);
        }

A teď malá otázka do pléna – co se mi na tomto kousku kódu nelíbí? Vadí mi tam tři věci, podotýkám, že aplikace je psaná v .NETu 2.0.

 

 

 

První vada na kráse

Metoda Replace je použitá špatně. Pokud už nám někdo dá string, kde bude sekvence znaků \’, nahrazení se provede a máme průšvih – v řetězci bude \\’ (nahrazujeme jen \’, i když v kódu jsou 2, to je kvůli C#, který za \ očekává nějaký speciální znak), což ale už ukončí řetězec, dvě lomítka za sebou se zinterpretují jako jedno a pak máme samotný apostrof. Takhle tedy ne.

Druhá vada na kráse

Ono vůbec použití metody Replace touto formou není vhodné. Za týden někdo přijde a řekne, že dělá v javascriptu v řetězcích problémy ten a ten znak, tak co programátor udělá? Dopíše to nějak takhle: …Translate(“něco”).Replace(“’”, “\\’”).Replace(“něco”, “něco jiného”). Za měsíc se zase někdo ráno vzbudí a řekne, že by se hodilo ještě nahradit tohle tímhle, a bude tam Replace třikrát.

I když je to jen jeden Replace, dělá už něco specifického, co se v budoucnu může potencionálně rozšiřovat. Proto by to mělo být zapouzdřeno v nějaké metodě, jejíž název odpovídá tomu, co to dělá – např. EscapeString nebo tak nějak. Osobně na tyhle věci kladu dost velký důraz a považuji je za to, co odděluje profesionála a začátečníka – schopnost myslet dopředu a počítat s místy, kde se bude v budoucnu možná něco přidělávat. Tohle není specifické pro escapování javascriptových stringů, dá se to zobecnit a použít v tisících případů. I když je tělo funkce na jeden řádek a mnohým může její vytvoření přijít zbytečné, v budoucnu se bude určitě hodit a je dobré to udělat hned, než až ve chvíli, kdy ten stejný Replace, který je sice sám o sobě triviální, máte na desítkách míst v celém projektu.

Třetí vada na kráse

Doslova ze židle mě zvedají poslední 4 řádky kódu – proč věci dělat jednoduše, když jdou dělat složitě. Pro registraci skriptu do stránky samozřejmě lze použít i tento kód, ale na to už v .NETu máme funkce. Co asi tak dělá funkce Page.ClientScript.RegisterClientScriptBlock? Navíc pokud by se čirou náhodou tato metoda spustila dvakrát? Dobře, to je dost nepravděpodobné, ale co když tento kus kódu někdo zkopíruje, použije ho jinde, a neuvědomí si, že se cílová metoda dvakrát spustit může? Stane se dost nepříjemná věc – tento blok kódu se ve stránce objeví dvakrát, což vyvolá chybu javascriptu.

Pro tento účel je právě vhodná funkce Page.ClientScript.RegisterClientScriptBlock a k ní přidružená Page.ClientScript.IsClientScriptBlockRegistered.

Jak to spravit?

První problém je na řešení imho dost náročný, pokud je aplikace psána v .NETu 2.0. Nejjednodušší je použít třídu novou třídu System.Web.Script.Serialization.JavaScriptSerializer, která umí serializovat do formátu JSON, což je de facto kód v javascriptu. Pokud vytvoříte její instanci a metodě Serialize předáte proměnnou typu string, na výstup dostanete správně zaescapovaný řetězec v Javascriptu. Tato třída je ale bohužel v .NET 3.5. Pokud chcete pořádné řešení v .NETu 2.0, mělo by stačit nejprve všechna zpětná lomítka zdvojit, a pak už můžete apostrofy nahradit. Snad to potom bude fungovat, ruku do ohně bych za to nedal.

Řešením druhého problému, tedy opakování Replace, je samozřejmě udělat na to vlastní metodu, která se o správné zakódování řetězce postará.

No a třetí problém se vyřeší pomocí již zmiňovaných funkcí RegisterClientScriptBlock a IsClientScriptBlockRegistered. Jejich kombinace navíc zajistí, že ať se metoda, která je generuje, zavolá, kolikrát chce, bude výsledný kód ve stránce jen jednou.

Perlička na závěr

Mimo jiné, open source projekty jsou opravdu zábavné a nepřestávají mě překvapovat. Zhruba před měsícem jsem potřeboval použít jedno nejmenované open source dílo v jednom projektu (použít open source projekt = dobastlit si tam 50% funkcionality, aby to nějak fungovalo, a výhledově si to sám přepsat) a měl jsem co dělat, abych nespadnul ze židle.

Představte si následující situaci – máte proměnnou a potřebujete na ní zavolat nějakou metodu, ale v té proměnné může být hodnota null. Jak to ošetřit? No jednoduše, přece tu metodu zavolám a odchytím NullReferenceException. Člověka, který tohle napsal, by měli bez pardonu zastřelit – řešit tyhle věci výjimkou je stotisíckrát pomalejší než udělat jeden pitomý if, který je navíc kratší. Co se totiž stane ve skutečnosti? V proměnné je fyzicky hodnota null, tedy odkaz na adresu 0x00000000. Když na ní něco zavoláte, stane se zjednodušeně asi toto: kvůli rychlosti a výkonu se před zavoláním metody automaticky nekontroluje, jestli je v proměnné nula, ale prostě to normálně zavolá. Samozřejmě díky tomu šáhneme do nepovolené paměti, což na procesoru vyvolá přerušení, načež následuje dost časově náročné zpracování této výjimečné události operačním systémem. To celé probublá až někam do runtime .NETu, který si řekne, že to asi byla NullReferenceException, a pak začne celá mašinérie spojená s obsluhou jedné hloupé výjimky. Jeden hloupý if je nesrovnatelně rychlejší, je to pár taktů procesoru. Obsluha výjimky mohou být klidně tisíce taktů.

Schválně jsem udělal takový malý testík – obě dvě situace jsem zkusil nasimulovat 10000x a měřil jsem, jak dlouho trvají, pomocí přesné třídy System.Diagnostics.Stopwatch. Test chytáním výjimky trval 1289042 ticků, test prostou podmínkou zabral 85! Ať už je tick jakkoliv dlouhý, rozdíl je propastný, if je asi 15000x rychlejší.

            string a = null;
            Stopwatch sw = new Stopwatch();
            sw.Start();
            for (int i = 0; i < 10000; i++)
            {
                try
                {
                    a = a.ToString();
                }
                catch (NullReferenceException nre) { }
            }
            sw.Stop();
            Literal1.Text = sw.ElapsedTicks.ToString() + "<br />";
            
            sw.Reset();
            sw.Start();
            for (int i = 0; i < 10000; i++)
            {
                if (a != null)
                    a = a.ToString();
            }
            sw.Stop();
            Literal1.Text += sw.ElapsedTicks.ToString();

Takže než budete něco ošetřovat z lenosti tak, že kolem celého bloku odchytíte všechny výjimky, vzpomeňte si na tento článek a honem to opravte. Z výše uvedeného textu nevyplývá, že by se výjimky neměly používat, protože jsou pomalé. Někde je jejich použití nevyhnutelné a pokud se používají rozumně, jsou k nezaplacení a pořádně se bez nich programovat nedá. Je ale nutné rozlišovat, kdy se používat mají, a kdy ne.

 

hodnocení článku

1 bodů / 1 hlasů       Hodnotit mohou jen registrované uživatelé.

 

Nový příspěvek

 

Diskuse: Galerie programátorských hříchů (díl 1.)

Jenom tak pro pobaveni - clovek ktery vymyslel 'null' toho zjevne docela lituje :-). Viz.: http://lambda-the-ultimate.org/node/3186

Tomas Petricek

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Diskuse: Galerie programátorských hříchů (díl 1.)

A není ještě trochu hřích nepoužít WITH konstrukci, když se tolikrát odkazuji na stále stejný objekt? Mám představu, že tím říkám kompilátoru, pozor tuhle referenci si někam schovej, ještě se ti bude hodit než ji stále hledat v paměti znovu a znovu.

Ale nevím, možná to už je schopen vyřešit (zoptimalizovat) kompilátor sám?

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Ano máte pravdu, ale dementní C# With konstrukci neumí...

nahlásit spamnahlásit spam 0 odpovědětodpovědět

tak to jsem trochu v šoku:)

BTW: Proč se každej honí za C#? Plné OOP a ukazatele ma VB už taky a tak nějak neznám důvod. Ale je pravda, že v C# neumim napsat ani řádku takže o C# nic nevím:)

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Ukazatele ve VB.NET? Mohl byste, prosím, doplnit mé vzdělání? Jak ve VB.NET uděláte ukazatel?

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Asi jsem se špatně vyjádřil, nemohu jej vytvořit, ale použít jako Address Of? Ale ted tuším, že asi pletu reference a ukazatel tak jak jej zná C#.

nahlásit spamnahlásit spam 0 odpovědětodpovědět

AddressOf je jenom syntaktická klička pro vytvoření delegátu. S trochou tolerance by se dalo říct, že delegát je ukazatel na metodu.

nahlásit spamnahlásit spam 1 / 1 odpovědětodpovědět

Pointer ve VB.NET představuje struktura System.IntPtr a pracovat se s tím dá pomocí System.Runtime.InteropServices.Marshal.* ale je to dost komplikované a kromě spolupráce s Windows API to nemá prakticky žádné využití. Mechanismus pointerů v C# jsem nezkoumal, ale předpokládám že to bude taky něco okolo IntPtr.

nahlásit spamnahlásit spam 0 odpovědětodpovědět

ano máte pravdu, jednou jsem zkoušel použít tenhle příklad: http://www.dotnetbips.com/articles/44bad...

Ale ve výsledku jsem zjistil, že mi to nic moc nového než AdrOf nepřineslo.

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Pokud si dovolíte toto tvrdit, potom vůbec nechápete funkci ukazatele a klíčového slova AddressOf...

nahlásit spamnahlásit spam 1 / 1 odpovědětodpovědět

to je dost dobře možné. využívám ji spíše intuitivně...

2 autors: co takhle nějaký článek, který v tom udělal jasno?

nahlásit spamnahlásit spam -1 / 1 odpovědětodpovědět

když tak nad tím přemýšlím, tak jsem AdrOf použil jen při "linkování" eventu (add handler)

nahlásit spamnahlásit spam 0 odpovědětodpovědět

AddressOf vytvoří delegáta (což je něco, co uvnitř pointer obsahuje, ale jako takový to pointer není).

Co se týče IntPtr, tak to pointer je, ale jak už psal pan Linhart, nedá se ve VB.NET použít na moc věcí. "Opravdové" pointery umí akorát C# v unsafe blocích.

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Použít With blok tady moc nepomůže, jediné, co je společné, je instance StringBuilderu, ale použitím With bloku by se moc neušetřilo. Navíc C# With blok nemá, takže tím se to řeší.

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Diskuse: Galerie programátorských hříchů (díl 1.)

Opensource věci jsou na humusácký kód specialita, ale občas se děsím i při mých výletech do nitra .NET Frameworku (i když tam to většinou není o prasáctví, ale o zbytečné komplikovanosti kódu)...

nahlásit spamnahlásit spam 0 odpovědětodpovědět
                       
Nadpis:
Antispam: Komu se občas házejí perly?
Příspěvek bude publikován pod identitou   anonym.

Nyní zakládáte pod článkem nové diskusní vlákno.
Pokud chcete reagovat na jiný příspěvek, klikněte na tlačítko "Odpovědět" u některého diskusního příspěvku.

Nyní odpovídáte na příspěvek pod článkem. Nebo chcete raději založit nové vlákno?

 

  • Administrátoři si vyhrazují právo komentáře upravovat či mazat bez udání důvodu.
    Mazány budou zejména komentáře obsahující vulgarity nebo porušující pravidla publikování.
  • Pokud nejste zaregistrováni, Vaše IP adresa bude zveřejněna. Pokud s tímto nesouhlasíte, příspěvek neodesílejte.

přihlásit pomocí externího účtu

přihlásit pomocí jména a hesla

Uživatel:  
Heslo:  

zapomenuté heslo

 

založit nový uživatelský účet

zaregistrujte se

 
zavřít

Nahlásit spam

Opravdu chcete tento příspěvek nahlásit pro porušování pravidel fóra?

Nahlásit Zrušit

Chyba

zavřít

feedback