Ir al contenido principal

Programación en C: Código sucio vs. Código optimizado

He andado liado con exámenes y emocionado con varias ramas del conocimiento {nota mental: hacer post sobre la pasión, el aprendizaje, la ampliación del conocimiento y porqué las mates molan (y sí, yo las odiaba y suspendía en la ESO y Bachillerato)}.

Una de esas cosillas productivas que he ido haciendo es estudiar el curso CS50x de Harvad, que, cómo su nombre indica, va de "Introducción a las Ciencias de la Computación".

Bien, el caso es que uno de los ejercicios consistía en crear un programa basado en la línea de comandos con el que encriptar un texto usando el cifrado de Vigenère. Yo, ni corto ni perezoso, sin mirarme ni practicar nada el temario del curso, me puse a picar código, cómo si no hubiera mañana, sin pararme a reflexionar y abordar el problema de formas diferentes. El resultado lo podéis ver a continuación (perdonadme la ausencia casi total de comentarios). {Nota mental: subir código a GitHub}

#include <stdio.h>
#include <cs50.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

//This program ciphers a text using Vigenere cipher

int main(int argc, string argv[])
{
    if (argc != 2) //check number of arguments
    {
        printf("Use of the program: ./vigenere.c 'keyword'. You must use one argument.\n");
        return 1;
    }
    else
    {
        string k = argv[1]; 
        int l = strlen(k);
        int c = 0;
        for (int x = 0; x < l; x++)
        {
            if (! isalpha(k[x]))
            {
                c++;
            }
        }
        if (c > 0) 
        {
            printf("Keyword can only include alphabetical characters.\n");
            return 1;
        }
    }

    string text = GetString();
    int length = strlen(text);

    char plain[length];
    strcpy(plain,text);
    string key = argv[1];
    int key_len = strlen(key);
    int t = 0;
    int i;
    
    for (i = 0; length > i; i++) //first step: put the key
    {
        if (t >= key_len)
        {
            t = 0;
            if (isupper(text[i]) || islower(text[i]))
            {
                
                text[i] = key[t];
                t++;
            }
            else
            {
                text[i] = text[i];
            }
        }
        else
        {
            if (isupper(text[i]) || islower(text[i]))
            {
                
                text[i] = key[t];
                t++;
            }
            else
            {
                text[i] = text[i];
            }
        }
        
    }
    
    int exc;
    
    for (i = 0; length > i; i++) //second step: cipher the key
    {
        
        if (! isalpha(text[i]))
        {
            printf("%c", text[i]);
        }
        else if (isupper(text[i]))
        {
            exc = plain[i] + (text[i] - 65);
            if (exc < 91)
            {
                printf("%c", exc);
            }
            else
            {
                printf("%c", exc - 26);
            }
        }
        else if (islower(text[i]))
        {
            exc = plain[i] + (text[i] - 97);
            if (exc < 123)
            {
                printf("%c", exc);
            }
            else
            {
                printf("%c", exc - 26);
            }
        }
    }
    
    printf("\n");
    
    return 0;
}

Hasta aquí mi desastre. Más tarde busqué ejemplos de este mismo ejercicio, creados por otros usuarios. Y encontré la versión perfecta (supongo que de ahí viene la expresión "at master") de este señor:


/****************************************************************************
 * vigenere.c                                                     
 *                                                              
 * Edward M. Poot  
 * edwardmp@gmail.com                                             
 *                                                              
 * Encode an user supplied string using a Vigenere style cipher.              
 * Alphabetical key is being entered by the user as a command line argument.
 *
 * Please note I already used some char* pointers to get familiar.             
 ***************************************************************************/

#import <stdio.h>
#import <cs50.h>
#import <string.h>
#include <ctype.h>

// function prototype
string VigenereCipher(string plain, string key);

int main(int argc, string argv[])
{   
    // typecast input string to integer if 1 argument is given, else return 0 so program exits in the next if statement
    string key = argv[1];
    
    // if more than 2 parameters (first being ./vigenere, second the key)
    // are supplied abort the program
    if (argc != 2) {
        printf("Usage: vigenere <key>\n");
        printf("Wrong number of parameters, string or negative number supplied. Aborting.\n");
        
        // stop executing program
        return 1;
    }
    
    // loop through all the keys in the key and check if they are all alphabetical
    for(int i = 0, j = strlen(key); i < j; i++)
    {
        if (!isalpha(key[i]))
        {   
            printf("Usage: vigenere <key>\n");
            printf("Non-alphabetic characters given. Aborting.\n");
            
            // impossibile to do Vigenere cipher using non-alphabetical chars, abort.
            return 1;
        }
    }
    
    string plainText;
    
    do {
        plainText = GetString();
    }
    while (strlen(plainText) == 0); // keep asking for input until user enters at least 1 char
    
    // cipher the plainText
    string result = VigenereCipher(plainText, key);
    
    // print the result back to the user
    printf("%s\n", result);
    
    return 0;
}

/*
 * Ciphers given string using Vigenere cipher, alphabetical letters shifted by wrapping the key letters around cyclically if needed
 * @param string Input that is being ciphered
 * @param string Use this string to cyclically wrap around the input  
 */
string VigenereCipher(string plain, string key)
{
    // calculate length of key used later
    // define it here so strlen() won't be run each time the for loop runs
    int keyLength = strlen(key);
    
    // initialize a variable that will hold ciphered string, needs to be same length as plain string
    char* cipher = plain;
    
    // loop through all characters of the plain text string
    for (int i = 0, j = 0, n = strlen(plain); i < n; i++)
    {   
        // if alphabetic proceed with ciphering, else return normal character
        if (isalpha(plain[i])) {
            
            // calculate ASCII code for the key position (j), wrap around beginning of keyword when end of keyword is reached
            char keyReferenceValue = (isupper(key[j % keyLength])) ? 'A' : 'a';
            
            // calculate ASCII code of the first letter of alphabet depending on upper- or lowercase
            char referenceValue = (isupper(plain[i])) ? 'A' : 'a';
            
            // calculate cipher letter using formula ci = (pi + kj) % 26, then convert to right ASCII character number
            cipher[i] = ((plain[i] - referenceValue + (key[(j % keyLength)] - keyReferenceValue)) % 26) + referenceValue;
            
            // printf("%c %i %i %i %i %i\n", cipher, j, strlen(key), key[(j % keyLength)], referenceValue, rotate);
           
            // increment key position (j) so next letter in key is used
            j++;
        }
        else {
            // output all non-alphabetical chars normally
            cipher[i] = plain[i];
        }
    }
    
    // return ciphered string
    return cipher;
}

108 líneas de código, contando comentarios a montones. El mío ocupa 115 sin comentarios. Este hombre ha usado punteros, ¡y yo ni me acordaba de que existían! ¡Y funciones prototipo! 
Espero que os sirva la entrada como ejemplo de pensar las cosas antes de hacerlas, y de saber deternse a estudiar los contenidos.

Comentarios