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