sorting failed error

maarten van damme maartenvd1994 at gmail.com
Mon Jul 30 15:30:59 PDT 2012


2012/7/31 Timon Gehr <timon.gehr at gmx.ch>:
> I realize that the code is just for fun, but I have some comments:
>
> - don't .dup stuff just in order to index directly. it never has
> any effect other than performance degradation. (this could be a
> compiler warning)
>
> - crossOver and mutate mutate their parameters in-place. I assume this
> is by mistake. reproduce effectively inserts every element a couple
> times because of this -- this is why the bug manifests reliably.
> (const, immutable annotations?)
>
> - make use of $
>
> int from=uniform(0,victim.dna.length);
> int to=uniform(0,victim.dna.length);
> swap(victim.dna[from],victim.dna[to]);
>
> =>
>
> swap(victim.dna[uniform(0,$)],
>      victim.dna[uniform(0,$)]);
>
> - sort is in-place
>
> workers=array(sort!(fitnessCompare)(workers));
>
> =>
>
> sort!fitnessCompare(workers)
>
> - mark local functions static if they do not need to access the
> enclosing context.
>
> - use size_t for array iteration variables (if necessary)
>
>
> for(int x=0;x<cities.length-1;x++)
>     travelled+=distance(cities[x],cities[x+1]);
>
> =>
>
> for(size_t x=1;x<cities.length;x++)
>     travelled+=distance(cities[x-1],cities[x]);
>
> I also removed the subtraction from the array length. This would be
> correct in this case because cities.length>=2 is guaranteed, but it is
> an error prone pattern.
>
> - another way to do it is to drop the loop completely and to use
> ranges instead:
>
> return zip(cities,cities.drop(1))
>       .map!(a=>distance(a.expand))
>       .reduce!"a+b";
>
>
> The benefit is that this is now obviously correct.
>
>
> You might also want to consider not building up the cities array
> and computing the terms at the boundary explicitly instead:
>
> return distance(city(0,0), victim.dna[0]) +
>        distance(victim.dna[$-1], city(0,0)) +
>        zip(victim.dna, victim.dna.drop(1))
>       .map!(a=>distance(a.expand))
>       .reduce!"a+b";
>
> The goal is to craft the shortest code possible that is still
> efficient and easy to follow.
>
> - type deduction?
>
>
> further comments whose application does not lead to immediate benefit:
>
> - in contracts are better specified in their dedicated section to push
> the requirements onto the caller.
>
> - consider for(;;) as a means for indefinite looping.
>
> - the convention is upper case for type names

Thank you very much for this criticism, I'm relatively new to
programming and these mistakes/points are the kind off things you
can't learn from reading a book or two.

I have one little question about one of the last points though
why use for(;;)?
As far as I can tell this simply reduces readability from while(true).
Is there any reason for this?


More information about the Digitalmars-d-learn mailing list