Awful code

Member
Posts: 45
Joined: 2006.11
Post: #1
I just finished writing a function that is, I think, one of the most awful pieces of code I've ever written, and I thought I'd share it with you guys.

I particularly like the set<string>::iterator j = i->second.begin()->second.begin();Wacko


Code:
/**
* This function takes a mapping of the dataset which maps
* for each station and each date, a set of times for which
* there is valid data (ie "good times"). It then generates a mapping which maps
* for each station, a set of good times that exist for all dates
* in the dataset for that station.
* \param dmap This is the original dataset mapping, generated as the
*    data was processed. it's in the form of:
*    map<station, map<date, set<good times> > >
* \param goodNums This is the output mapping of stations to a set
*    of good times that are present on all dates in the dataset.
*    it is in the form of:
*    map<station, set<good times> >
*/
void ProcessDataMap(map<string, map<string, set<string> > >& dmap,
                    map<string, set<string> >& goodNums)
{
    map<string, map<string, set<string> > >::iterator i = dmap.begin();
    goodNums.clear();
    
    // for each station
    while(i != dmap.end())
    {
        set<string> good;
        
        if(i->second.size() > 0)
        {
            // add the first date's set of good numbers to the good pile
            set<string>::iterator j = i->second.begin()->second.begin();
            while(j != i->second.begin()->second.end())
            {
                good.insert(*j); // add the number
                j++;    
            }
            i++;
        
        
            // now for the rest of dates' lists of good numbers, apply the following rule
            // if a number in the good pile is not in the current list, remove it from the good pile
            map<string, set<string> >::iterator k = i->second.begin();
            k++; // skip the first one, we got it already
            while(k != i->second.end())
            {
                // apply rule 1)
                set<string>::iterator g = good.begin();
                while(g != good.end())
                {
                    // if the number isn't in the list, remove it from the good pile
                    if(k->second.find(*g) == k->second.end())        
                    {
                        set<string>::iterator next = g; next++; // save the next spot
                        good.erase(g); // remove it from the good pile
                        g = next;
                    }
                    else
                        g++; // *g is in both the current date's good times and the good pile
                }
            }
        }
    }
}
Quote this message in a reply
Sage
Posts: 1,403
Joined: 2005.07
Post: #2
LOL Some tips.

1) Forget comments, they just get in the way of the code.
2) Needs more STL (when in rome)
3) Far too long, you could try to squeeze most of it onto one line
4) conventional use of iteration constructs, spice it up with some #defines

heres some bad code i've written:
Code:
char ** read_lines(char * filename) {
  char ** output, * p, ** q;
  struct stat file_stat;
  FILE * fptr = fopen(filename, "r");
  
  *(q = (output = malloc(fread(p = malloc(fstat(fileno(fptr), &file_stat) + file_stat.st_size), 1, file_stat.st_size, fptr)*sizeof(char *)))) = p;
  do do if(*p == '\n') *p = '\0'; while(*p++); while((*q++ = p) && *p);
  *q++ = NULL;
  
  return output;
}

Sir, e^iπ + 1 = 0, hence God exists; reply!
Quote this message in a reply
Member
Posts: 161
Joined: 2005.07
Post: #3
char ** output? fopen? Those things are still readable!

Amateurs. Rasp
Quote this message in a reply
Moderator
Posts: 608
Joined: 2002.04
Post: #4
unknown Wrote:
Code:
char ** read_lines(char * filename) {
  char ** output, * p, ** q;
  struct stat file_stat;
  FILE * fptr = fopen(filename, "r");
  
  *(q = (output = malloc(fread(p = malloc(fstat(fileno(fptr), &file_stat) + file_stat.st_size), 1, file_stat.st_size, fptr)*sizeof(char *)))) = p;
  do do if(*p == '\n') *p = '\0'; while(*p++); while((*q++ = p) && *p);
  *q++ = NULL;
  
  return output;
}
Cry

...some more characters...
Quote this message in a reply
Post Reply