Too complicated code for generating a random string?

monarch_dodra monarchdodra at gmail.com
Sat Feb 23 11:17:56 PST 2013


On Saturday, 23 February 2013 at 19:08:04 UTC, jerro wrote:
>>    string randomString;
>>    {
>>        auto tmp = new char[](10);
>>        foreach(ref char c ; tmp)
>>            c = letters[uniform(0, letters.length)];
>>        randomString = cast(string)letters;
>>    }
>>
>> All those scope blocks I put in are not mandatory. At the end 
>> of the day, it is 4 lines of code. Very efficient, and the 
>> intent is crystal clear.
>
> Or you could simply do this:
>
> string randomString = iota(10).map!(_ => letters[uniform(0, 
> $)]).array;
>
> It's one line off code and the intent seems crystal clear to 
> me. It's also equally efficient. I've benchmark those to pieces 
> of code for various values of n with both DMD and GDC, using -O 
> -inline -release:
>
> auto tmp = iota(n).map!(_ => letters[uniform(0, $)]).array;
>
> ....
>
> auto tmp = new char[](n);
> foreach(ref char c ; tmp)
>     c = letters[uniform(0, letters.length)];
>
> For n=10 and when compiled with GDC, the first snippet actually 
> performed a little bit (about 10%) better, but for larger sizes 
> there was no significant difference.

In my defense, I had started writing that when the "1 liner" was 
still:

auto randomLetter = () => randomSample(letters, 1, 
letters.length).front;
writeln(iota(10).map!(_ => randomLetter()));

The "new" 1-liner is indeed good, and what I would actually use. 
I still think though that there is a point where you need to stop 
and think "is my 1 liner actually understandable and 
maintainable". In this first case, the answer (IMO), was no.

Now it is, so good for us.

BTW, I think the clearest remains my generator proposal:
string randomString =
     fastGenerator!(() => letters[uniform(0, $)]).take(9).array;

Any chance you could tell me how it fares in your bench?


More information about the Digitalmars-d mailing list