3
\$\begingroup\$

Is this the best way to implement a genetic algorithm framework? I'm just a student and wanted to know if I can improve the code that I have done. Some of the code I copied from the framework documentation:

namespace xx.xx.xx {
    class Program {


        static GrafoDistrito grafo;
        static int total;


        static void Main(string[] args) {

            using (var leitura = File.OpenText(@"C:\portugal.data")) {   

                string sTotal = leitura.ReadLine();
                total = Int32.Parse(sTotal);
                grafo = new GrafoDistrito(total);
                Random rnd = new Random();


                for (int i = 1; i <= total; i++) {

                    string line = leitura.ReadLine();               
                    string[] identificadores = line.Split(' ');
                    int id = Int32.Parse(identificadores[0]);
                    string descricao = identificadores[1];
                    Distrito distrito = new Distrito(id, descricao);
                    grafo.AdicionarVetor(distrito);
                    Console.WriteLine(identificadores[1]);
                }



                for (int j = 1; j <= total; j++) {

                    string line = leitura.ReadLine();
                    string[] identificadores = line.Split(' ');
                    int totalVis = identificadores.Length;
                    int DistAtual = Int32.Parse(identificadores[0]);



                    for (int u = 1; u < totalVis; u++) {

                        int id = Int32.Parse(identificadores[u]);
                        grafo.Connect(grafo.GetVertice(DistAtual), grafo.GetVertice(id));
                    }
                }



                var population = new Population();
                int[] arrayD = new int[total];



                for (int a = 0; a < total; a++) {
                    arrayD[a] = a + 1;
                }



                for (int u = 0; u < 100; u++) {

                    var chromosome = new Chromosome();
                    arrayD = arrayD.OrderBy(x => rnd.Next()).ToArray();



                    for (var j = 0; j < arrayD.Length; j++) {

                        chromosome.Genes.Add(new Gene(arrayD[j]));
                    }



                    chromosome.Genes.ShuffleFast();
                    population.Solutions.Add(chromosome);
                }



                var elite = new Elite(5);   //criação do operador elite
                var crossover = new Crossover(0.8);     
                var mutacao = new SwapMutate(0.02);     
                var gA = new GeneticAlgorithm(population, CalculateFitness);   



                //operators of Genetic Algorithm
                gA.Operators.Add(elite);
                gA.Operators.Add(crossover);
                gA.Operators.Add(mutacao);

                //run Genetic Algorithm
                gA.Run(Terminate);
            }
        }



        static bool Terminate(Population pop, int currentGeneration, long currentEvaluation) {

            return currentGeneration >= 400;
        }



        static double CalculateFitness(GAF.Chromosome chromossome) {

            int[] corArray = new int[chromossome.Genes.Count];
            int cor;



            foreach (Gene gene in chromossome.Genes) {

                int id = Convert.ToInt32(gene.RealValue);
                Distrito vertice = grafo.GetVertice(id);
                List<int> vizinhos = grafo.GetAresta(vertice).ToList();
                List<int> corVisivel = new List<int>();



                foreach (int a in vizinhos) {

                    int vizinhoCor = corArray[a - 1];
                    if (vizinhoCor > 0)
                        corVisivel.Add(vizinhoCor);
                }



                cor = 1;
                while (corVisivel.Contains(cor)) {
                    cor++;
                }



                corArray[id - 1] = cor;
            }



            int maxCor = 0;
            for (int i = 0; i < corArray.Length; i++) {
                if (corArray[i] > maxCor)
                    maxCor = corArray[i];         
            }



            return 1/ maxCor;   //o valor de retorno tem que ser entre 0 e 1. A melhor rota é o valor mais próximo de 1
        }



    }
}
\$\endgroup\$
  • \$\begingroup\$ Hi. Welcome to Code Review! It often helps if you add examples of the intended input and output to the question. You might get more feedback if you translated into English. As is, many of us won't be able to read the variable names and comments. \$\endgroup\$ – mdfst13 Jun 17 '16 at 23:45
1
\$\begingroup\$

You can make your code shorter and more readable with LINQ.

This:

int[] arrayD = new int[total];
for (int a = 0; a < total; a++) {
    arrayD[a] = a + 1;
}

Can be replaced with:

int[] arrayD = Enumerable.Range(1, total).ToArray();

This:

arrayD = arrayD.OrderBy(x => rnd.Next()).ToArray();
for (var j = 0; j < arrayD.Length; j++) {
    chromosome.Genes.Add(new Gene(arrayD[j]));
}

Can be replaced with:

chromosome.Genes.AddRange(arrayD.OrderBy(x => rnd.Next()));

Note that arrayD does not get shuffled each time. I'm not sure if that will affect the randomness.

This:

List<int> corVisivel = new List<int>();
foreach (int a in vizinhos) {
    int vizinhoCor = corArray[a - 1];
    if (vizinhoCor > 0)
        corVisivel.Add(vizinhoCor);
}

Can be replaced with:

List<int> corVisivel = vizinhos.Select(x => corArray[x - 1]).Where(x => x > 0).ToList();

This:

cor = 1;
while (corVisivel.Contains(cor)) {
    cor++;
}

Can be replaced with:

cor = Enumerable.Range(1, int.MaxValue).TakeWhile(x => corVisivel.Contains(x)).Count() + 1;

This:

int maxCor = 0;
for (int i = 0; i < corArray.Length; i++) {
    if (corArray[i] > maxCor)
        maxCor = corArray[i];         
}

Can be replaced with:

int maxCor = corArray.Max();

Why does Terminate() have three parameters but only use one of them? int cor; should be declared inside the foreach loop because it's not used outside of it. You can assign it on the same line that it's declared.

\$\endgroup\$
1
\$\begingroup\$
  1. It is better to use a constant variable for the file path, that way you don't need to look for that line if you wish to change the file path or name.

    using (var leitura = File.OpenText(@"C:\portugal.data"))
    
  2. The main function is just too long and hard to understand and maintain. Split it into a sub function. For example, the first function for reading from file AdicionarVetor() can be the code in the first loop.

  3. At the beginning of Main is a parser and you converting file to objects until the line:

    var population = new Population();
    

    The use of leitura should end before that line and all the using blocks should be transferred to another functions or even a dedicated object.

    Your Main should look like this:

    static void Main(string[] args) {
        grafo = LoadGrafo(@"C:\portugal.data");
        population = BuildPopulation();
    
        var elite = new Elite(5);   //criação do operador elite
        var crossover = new Crossover(0.8);     
        var mutacao = new SwapMutate(0.02);     
        var gA = new GeneticAlgorithm(population, CalculateFitness);   
    
        //operators of Genetic Algorithm
        gA.Operators.Add(elite);
        gA.Operators.Add(crossover);
        gA.Operators.Add(mutacao);
    
        //run Genetic Algorithm
        gA.Run(Terminate);
    }
    
  4. It is a bad practice to make grafo global. In this case, the program is building for a GenericAlgorithm, so this is ok. But it is up to you to extend this program. You must move grapfo and all that functionality to a dedicated class for preparing and running GenericAlgorithm, then you could manage the files and results from Main.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.