[alg] Slechtste programmeervoorbeelden deel 4

quote:

Davio schreef op dinsdag 21 mei 2013 @ 11:30:
Hihi, .NET gebruikt intern UTF-8 toch?

Dan zou het mijns inziens zinvoller zijn om een Encoding.UTF8.GetBytes te doen.
Naast dat je BitConverter.ToString(data) kunt doen natuurlijk.
Of eventueel de Base64-representatie als je het toch als string op wilt slaan.

Ik kan inderdaad geen reden bedenken waarom je ASCII zou verkiezen over UTF8, maar daar doelde ik nog nog geeneens op. Bitconverter en Base64 geven niet de gewenste (huidige manier van opslaan in db) format terug, en het moet aansluiten op de oude situatie.

Anyhow, er gaat zoveel mis in deze paar regels code. Ongeacht hoe je het zou willen encoden. StringBuilder versus String, Disposen, ToLower() op elk resultaat in de for-loop.
C#:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19

/// <summary> /// Convert current instance to a MD5 hash /// </summary> /// <param name="value">Current instance</param> /// <returns>MD5 hash</returns> public static string ToMD5(this string value) {     StringBuilder hexadecimal = new StringBuilder();     using (MD5 md5Hash = MD5.Create())     {         byte[] data = md5Hash.ComputeHash(Encoding.UTF8.GetBytes(value));         for (int i = 0; i < data.Length; i++)             hexadecimal.Append(data[i].ToString("x2"));     }     return hexadecimal.ToString().ToLower(); }

Lijkt mij al een stuk mooier. Nog meer suggesties?

quote:

Tarabass schreef op dinsdag 21 mei 2013 @ 12:07:
Lijkt mij al een stuk mooier. Nog meer suggesties?

Weet niet waar het voor is, maar md5 is niet secure als het voor signen is oid. En aangezien C# .NET gewoon ingebouwde SHA varianten heeft.

(en is die 'using' nou echt nodig? Dat je het een regel eerder opruimt?)

Zoijar wijzigde deze reactie 21-05-2013 12:34 (22%)

quote:

Zoijar schreef op dinsdag 21 mei 2013 @ 12:32:
[...]

Weet niet waar het voor is, maar md5 is niet secure als het voor signen is oid.

MD5 kan je nog uitstekend gebruiken voor vele andere zaken.
Ik gebruik het in een app om dubbele bestanden te vinden, zal vanavond eens kijken hoe ik mijn md5 genereer.

quote:

IStealYourGun schreef op dinsdag 21 mei 2013 @ 12:35:
MD5 kan je nog uitstekend gebruiken voor vele andere zaken.

Vandaar ook mijn specifieke bewoording in die zin

Ik zou de methode ook niet "ToMD5" noemen. MD5 genereert een byte hash van een input in bytes,

ToMD5String zou ik dan toepasselijker vinden.

Verder kun je het best zonder een StringBuilder doen, dat is volgens mij één van de langzaamste implementaties.
Zie ook: http://stackoverflow.com/...e-versa/14333437#14333437

Ik werd toch even nieuwsgierig naar de performance van verschillende manieren om een string om te zetten naar MD5. Ik heb drie implementaties genomen, en ToMD5_3 is het snelste. Bij 1.000.000 iteraties, op een i7-2600k:

ToMD5_1: 7075ms
ToMD5_2: 6300ms
ToMD5_3: 5752ms

C#:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102

using System; using System.Diagnostics; using System.Security.Cryptography; using System.Text; namespace ConsoleApplication1 {     static class StringExtensions     {         public static string ToMD5_1(this string value)         {             StringBuilder hexadecimal = new StringBuilder();             using (MD5 md5Hash = MD5.Create())             {                 byte[] data = md5Hash.ComputeHash(Encoding.UTF8.GetBytes(value));                 for (int i = 0; i < data.Length; i++)                     hexadecimal.Append(data[i].ToString("x2"));             }             return hexadecimal.ToString().ToLower();         }         public static string ToMD5_2(this string value)         {             byte[] data;             using (MD5 md5 = MD5.Create())             {                 data = md5.ComputeHash(Encoding.UTF8.GetBytes(value));             }             return BitConverter.ToString(data).ToLower().Replace("-", string.Empty);         }         public static string ToMD5_3(this string value)         {             byte[] data;             using (MD5 md5 = MD5.Create())             {                 data = md5.ComputeHash(Encoding.UTF8.GetBytes(value));             }             return data.ToHex().ToLower();         }     }     static class ByteExtensions     {         private const string hexChars = "0123456789ABCDEF";         public static string ToHex(this byte[] bytes)         {             int length = bytes.Length * 2;             char[] hex = new char[length];             int byteIndex = 0;             for (int charIndex = 0; charIndex < length; charIndex += 2)             {                 byte byteValue = bytes[byteIndex++];                 hex[charIndex] = hexChars[byteValue / 16];                 hex[charIndex + 1] = hexChars[byteValue % 16];             }             return new string(hex);         }     }     class Program     {         static void Main(string[] args)         {             int iterations = 1000000;             Stopwatch sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_1();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_2();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_3();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             Console.ReadLine();         }     } }

quote:

Zoijar schreef op dinsdag 21 mei 2013 @ 12:32:
[...]

(en is die 'using' nou echt nodig? Dat je het een regel eerder opruimt?)

Strict genomen niet (je krijgt er geen foutmelding van), maar hij implementeert IDisposable via HashAlgorithm.

Hmm, interessant! De using is imo een stukje good practice. Gewoonte dus.

Dat de byte[] naar char[] zo snel is verbaast me eigenlijk helemaal niet. Echter een benadering die ik niet snel gekozen zou hebben. Maar dit is nu het leuke van programmeren!

Het verbaast me overigens dat die stringbuilder en bitconverter zo 'traag' zijn. Nooit geweten

[edit]
Het is dus niet de bitconvert die traag is, maar het replacen. Zonder ".ToLower().Replace("-", string.Empty)" is de bitconverter toch een duidelijke winnaar (scheelt 2/3 seconden).

Tarabass wijzigde deze reactie 21-05-2013 13:24 (21%)

Misschien moet je een capacity meegeven aan je stringbuilder ctor, dan hoeft hij zeker geen reallocation te doen.

* Zoijar is C# noob en redeneert alles vanuit C++/STL

i.e.:
code:

public static string ToMD5_1(this string value)
        {

            MD5 md5Hash = MD5.Create();
            byte[] data = md5Hash.ComputeHash(Encoding.UTF8.GetBytes(value));

            StringBuilder hexadecimal = new StringBuilder(data.Length * 2);

            for (int i = 0; i < data.Length; i++)
                hexadecimal.Append(data[i].ToString("x2"));
            }

            return hexadecimal.ToString().ToLower();
        }

Time dat eens?

quote:

Cyphax schreef op dinsdag 21 mei 2013 @ 13:02:
Strict genomen niet (je krijgt er geen foutmelding van), maar hij implementeert IDisposable via HashAlgorithm.

Oh, ik denk dat ik me dan vergis in hoe C# werkt. Ik nam aan dat als hij uit scope ging sowieso die dispose wordt aangeroepen. Wie heeft er ook GC nodig als je RAII en smart pointers hebt

Zoijar wijzigde deze reactie 21-05-2013 13:41 (103%)

quote:

Tarabass schreef op dinsdag 21 mei 2013 @ 13:15:
Hmm, interessant! De using is imo een stukje good practice. Gewoonte dus.

Dat de byte[] naar char[] zo snel is verbaast me eigenlijk helemaal niet. Echter een benadering die ik niet snel gekozen zou hebben. Maar dit is nu het leuke van programmeren!

Het verbaast me overigens dat die stringbuilder en bitconverter zo 'traag' zijn. Nooit geweten

[edit]
Het is dus niet de bitconvert die traag is, maar het replacen. Zonder ".ToLower().Replace("-", string.Empty)" is de bitconverter toch een duidelijke winnaar (scheelt 2/3 seconden).

Tja, het vervelende van de zaak is dat het daarmee wel leesbaarder wordt en er is iets voor te zeggen om die implementaties daarom te kiezen, hangt er ook vanaf hoe vaak zo'n methode wordt uitgevoerd en op welk niveau het zit.

In zo'n StringExtensions-class is het van de categorie: Een keer programmeren en je hoeft er voor de rest niet meer naar te kijken. Dan is de meest snelle, minder leesbare, oplossing prima.

Capacity op de stringbuilder maakt niets uit. Heb één met en één zonder gedeclareerd en ze wisselen elkaar af (op snelheid).

Ik denk dat ik voor de bitconverter zou kiezen. Mede voor de leesbaarheid. Qua performance ligt hij dicht bij de 'chararray' methode, maar qua leesbaarheid is hij zelfs beter dan de stringbuilder

code:

$tax = number_format(($aInvoice['invoice_define'] == 'C' ? -1 * $aRule['price']['price'] - $aRule['price']['discount'] : ($aRule['price']['price'] - $aRule['price']['discount'])), 2, ',', '');

Oh en de ouput was ook nog eens verkeerd

quote:

JapyDooge schreef op dinsdag 21 mei 2013 @ 16:20:
[..] verkeert [..]

quote:

Patriot schreef op dinsdag 21 mei 2013 @ 16:49:
[...]

C#:

1 2 3 4 5 6 7 8 9

public string Status { get; set; } public bool IsGoedgekeurd {     get     {         return this.Status == "Goedgekeurd";     } }

Alex) wijzigde deze reactie 24-05-2013 14:57 (6%)

Komt vaker voor, met name bij dingen waarbij zo'n string uit een externe bron komt

Al is het semantisch natuurlijk beter de bool op te slaan en die vergelijking in de setter te doen

Tsja, maar dat is hier niet het geval. Het datamodel zit in hetzelfde project als de rest van de applicatie. De status kan volgens de broncode die ik heb gevonden drie waardes hebben: Onbehandeld, Goedgekeurd en Afgewezen. Toch staat het als varchar(100) in de database.

Een enum (opgeslagen als short) of zelfs een nullable boolean zou al een stuk beter zijn geweest dan dit gedrocht.

Alex) wijzigde deze reactie 24-05-2013 15:13 (12%)

Je vergeet de status "Mwah, het is nog niet goedgekeurd maar we zijn er wel mee bezig en misschien passen we dit over 2 weken aan" die er ongetwijfeld in de toekomst in moet. Dat was je niet gelukt met een nullable boolean .

quote:

Alex) schreef op vrijdag 24 mei 2013 @ 15:12:
Tsja, maar dat is hier niet het geval. Het datamodel zit in hetzelfde project als de rest van de applicatie. De status kan volgens de broncode die ik heb gevonden drie waardes hebben: Onbehandeld, Goedgekeurd en Afgewezen. Toch staat het als varchar(100) in de database.

Een enum (opgeslagen als short byte) of zelfs een nullable boolean zou al een stuk beter zijn geweest dan dit gedrocht.

Als je dan toch al de moeite neemt om GEEN int te gebruiken...

quote:

Davio schreef op vrijdag 24 mei 2013 @ 15:20:
[...]

Als je dan toch al de moeite neemt om GEEN int te gebruiken...

Even vergeten, als byte kan inderdaad ook.

Of we doen het als ntext(MAX)...

quote:

Alex) schreef op vrijdag 24 mei 2013 @ 15:22:
[...]

Even vergeten, als byte kan inderdaad ook.

Of we doen het als ntext(MAX)...

Ja, maar ntext gaat altijd zo zeuren als je er een distinct overheen gooit.

quote:

Davio schreef op vrijdag 24 mei 2013 @ 15:24:
[...]

Ja, maar ntext gaat altijd zo zeuren als je er een distinct overheen gooit.

SQL:

1 2	SELECT DISTINCT(CONVERT(Status as nvarchar(MAX)) AS AlleStatussen  ORDER BY AlleStatussen ASC

Zeuren? Hoezo?

Ja, maar daar is het geen ntext!
Niet meer althans.

Davio wijzigde deze reactie 24-05-2013 15:28 (26%)

quote:

Davio schreef op vrijdag 24 mei 2013 @ 15:28:
Ja, maar daar is het geen ntext!
Niet meer althans.

En snel is het ook niet meer.

Maar ach, dan gooi je ieder jaar de database leeg, dan is het weer een maandje snel.

* heeft zulke code/werkwijze wel eens gezien. Verschrikkelijk.

quote:

ThomasG schreef op dinsdag 21 mei 2013 @ 12:57:
Ik werd toch even nieuwsgierig naar de performance van verschillende manieren om een string om te zetten naar MD5. Ik heb drie implementaties genomen, en ToMD5_3 is het snelste. Bij 1.000.000 iteraties, op een i7-2600k:

ToMD5_1: 7075ms
ToMD5_2: 6300ms
ToMD5_3: 5752ms

C#:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102

using System; using System.Diagnostics; using System.Security.Cryptography; using System.Text; namespace ConsoleApplication1 {     static class StringExtensions     {         public static string ToMD5_1(this string value)         {             StringBuilder hexadecimal = new StringBuilder();             using (MD5 md5Hash = MD5.Create())             {                 byte[] data = md5Hash.ComputeHash(Encoding.UTF8.GetBytes(value));                 for (int i = 0; i < data.Length; i++)                     hexadecimal.Append(data[i].ToString("x2"));             }             return hexadecimal.ToString().ToLower();         }         public static string ToMD5_2(this string value)         {             byte[] data;             using (MD5 md5 = MD5.Create())             {                 data = md5.ComputeHash(Encoding.UTF8.GetBytes(value));             }             return BitConverter.ToString(data).ToLower().Replace("-", string.Empty);         }         public static string ToMD5_3(this string value)         {             byte[] data;             using (MD5 md5 = MD5.Create())             {                 data = md5.ComputeHash(Encoding.UTF8.GetBytes(value));             }             return data.ToHex().ToLower();         }     }     static class ByteExtensions     {         private const string hexChars = "0123456789ABCDEF";         public static string ToHex(this byte[] bytes)         {             int length = bytes.Length * 2;             char[] hex = new char[length];             int byteIndex = 0;             for (int charIndex = 0; charIndex < length; charIndex += 2)             {                 byte byteValue = bytes[byteIndex++];                 hex[charIndex] = hexChars[byteValue / 16];                 hex[charIndex + 1] = hexChars[byteValue % 16];             }             return new string(hex);         }     }     class Program     {         static void Main(string[] args)         {             int iterations = 1000000;             Stopwatch sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_1();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_2();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             sw = Stopwatch.StartNew();             for (int i = 0; i < iterations; i++)             {                 "thisisasillystringtodosomemagicon".ToMD5_3();             }             sw.Stop();             Console.WriteLine((sw.ElapsedMilliseconds).ToString());             Console.ReadLine();         }     } }

Maar als je bv een wachtwoord met MD5 (of welke andere hashing methode dan ook) genereert wil je juist dat je wat trager bent Gezien het feit dat je dan moeilijker kan brute-forcen.

Iets van mezelf. Net ineens like 'WTF waarom deed ik dat zo.'. Zomaar
PHP:

1 2 3 4 5 6 7 8

<?php $a = 0; foreach ($array as $arrayitem) { $a++; echo $item; if($a < count($array)) { echo 'some stuff in between'; } } ?>

Wat natuurlijk veel beter zo kan:
PHP:

1	<?php echo implode('somestuffinbetween', $array); ?>

quote:

sanderev66 schreef op vrijdag 24 mei 2013 @ 18:58:
[...]

Maar als je bv een wachtwoord met MD5 (of welke andere hashing methode dan ook) genereert wil je juist dat je wat trager bent Gezien het feit dat je dan moeilijker kan brute-forcen.

waarom zou dat helpen? Dan kan je beter een sleep inbouwen aan het eind van de validatie-methode, scheelt je processortijd=load.

quote:

MBV schreef op vrijdag 24 mei 2013 @ 19:41:
[...]

waarom zou dat helpen? Dan kan je beter een sleep inbouwen aan het eind van de validatie-methode, scheelt je processortijd=load.

Of exponentieel blokkeren.

Drie keer achter elkaar fout is 1 min. geblokkeerd, tien keer fout een dag, twintig keer fout een jaar.

quote:

MBV schreef op vrijdag 24 mei 2013 @ 19:41:
[...]

waarom zou dat helpen? Dan kan je beter een sleep inbouwen aan het eind van de validatie-methode, scheelt je processortijd=load.

Ik denk dat hij, hoewel daar in dit geval geen sprake van was, doelde op het feit dat het hashen best wel iets duurder mag zijn zodat bruteforcen meer tijd kost.

quote:

MBV schreef op vrijdag 24 mei 2013 @ 19:41:
[...]

waarom zou dat helpen? Dan kan je beter een sleep inbouwen aan het eind van de validatie-methode, scheelt je processortijd=load.

Volgens mij is dat niet bedoeld voor 'zodat het op je productieserver' trager is. Maar dat als men achter het algoritme komt en de tabel met hashes ligt op straat men niet snel een betreffende hash kan bruteforcen.

Niet het bruteforcen 'live' op de server.

ZpAz wijzigde deze reactie 24-05-2013 21:46 (4%)

quote:

F.West98 schreef op vrijdag 24 mei 2013 @ 19:06:
Iets van mezelf. Net ineens like 'WTF waarom deed ik dat zo.'. Zomaar
PHP:

1 2 3 4 5 6 7 8

<?php $a = 0; foreach ($array as $arrayitem) { $a++; echo $item; if($a < count($array)) { echo 'some stuff in between'; } } ?>

Wat natuurlijk veel beter zo kan:
PHP:

1	<?php echo implode('somestuffinbetween', $array); ?>

Hah, heel herkenbaar. Het is ook heel moeilijk om te omschrijven om te kijken of er een functie is. Zeker binnen php.

quote:

Patriot schreef op vrijdag 24 mei 2013 @ 21:10:
[...]


Ik denk dat hij, hoewel daar in dit geval geen sprake van was, doelde op het feit dat het hashen best wel iets duurder mag zijn zodat bruteforcen meer tijd kost.

Precies Maar ik wist niet precies waar de code voor diende

sanderev66 wijzigde deze reactie 24-05-2013 22:41 (8%)

quote:

F.West98 schreef op vrijdag 24 mei 2013 @ 19:06:
Iets van mezelf. Net ineens like 'WTF waarom deed ik dat zo.'. Zomaar
PHP:

1 2 3 4 5 6 7 8

<?php $a = 0; foreach ($array as $arrayitem) { $a++; echo $item; if($a < count($array)) { echo 'some stuff in between'; } } ?>

Wat natuurlijk veel beter zo kan:
PHP:

1	<?php echo implode('somestuffinbetween', $array); ?>

PHP zelf doet het zelf ook (ongeveer) zo. Het staat natuurlijk wel netter als je implode() gebruikt
C:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37

PHPAPI void php_implode(zval *delim, zval *arr, zval *return_value TSRMLS_DC) {     zval         **tmp;     HashPosition   pos;     smart_str      implstr = {0};     int            numelems, i = 0;     zval tmp_val;     int str_len;     numelems = zend_hash_num_elements(Z_ARRVAL_P(arr));     zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(arr), &pos);     while (zend_hash_get_current_data_ex(Z_ARRVAL_P(arr), (void **) &tmp, &pos) == SUCCESS) {         tmp_val = **tmp;         zval_copy_ctor(&tmp_val);         convert_to_string(&tmp_val);         smart_str_appendl(&implstr, Z_STRVAL(tmp_val), Z_STRLEN(tmp_val));         zval_dtor(&tmp_val);         if (++i != numelems) {             smart_str_appendl(&implstr, Z_STRVAL_P(delim), Z_STRLEN_P(delim));         }         zend_hash_move_forward_ex(Z_ARRVAL_P(arr), &pos);     }     smart_str_0(&implstr);     if (implstr.len) {         RETURN_STRINGL(implstr.c, implstr.len, 0);     } else {         smart_str_free(&implstr);         RETURN_EMPTY_STRING();     } }

Niet de hele functie, alleen de relevante dingen.

Pizzalucht wijzigde deze reactie 24-05-2013 23:02 (6%)

quote:

sanderev66 schreef op vrijdag 24 mei 2013 @ 22:41:
[...]

Precies Maar ik wist niet precies waar de code voor diende

Daarom moet je ook een moeilijker algoritme gebruiken dan MD5, de hackers zijn over het algemeen heel goed in het efficiënt implementeren van je algoritme

quote:

MBV schreef op zaterdag 25 mei 2013 @ 01:03:
[...]

Daarom moet je ook een moeilijker algoritme gebruiken dan MD5, de hackers zijn over het algemeen heel goed in het efficiënt implementeren van je algoritme

Dat staat op zich los van het gebruikte algoritme. Als we kijken naar PHP, dan kun je middels de hash_pbkdf2 functie ook een heel goedkoop hash-algoritme gebruiken, dan moet je alleen het aantal iteraties omhoog schroeven om dezelfde hoeveelheid work te behalen als een relatief duur algoritme.