Re: I'm a newbie. Is this code ugly?

From:

ytrembla@nyx.nyx.net (Yannick Tremblay)

Newsgroups:

comp.lang.c++

Date:

20 Jan 2010 18:12:44 GMT

Message-ID:

<1264011163.935846@irys.nyx.net>

In article <4b572d01$0$1103$4fafbaef@reader4.news.tin.it>,
io_x <a@b.c.invalid> wrote:

"io_x" <a@b.c.invalid> ha scritto nel messaggio
news:4b57269b$0$1132$4fafbaef@reader1.news.tin.it...

"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
news:4iB21VKQpuVLFw7l@baesystems.com...

In message <4b56d0c2$0$828$4fafbaef@reader5.news.tin.it>, io_x
<a@b.c.invalid>
writes

"gert" <27hiro@googlemail.com> ha scritto nel messaggio
news:e4951ec7-f09b-4b74-9cf8-a7a9e19e400c@22g2000yqr.googlegroups.com...

I'm using a class which can sinksort an array of it's own objects and
an example T class, which can have names and stuff...
I was in doubt about what to do about nearly any line, so I would love
any of your recommendations...

what about this?

Horrible.

Monstruosity!

but i can debug all single bit of that

You probably can, that's still awful code.

This code works, after a fashion, because those strings are all literals.
What
would happen if you were reading values from a file?

no problem, i make local copy

hope not much wrong

#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#define R return
using namespace std;

char* faiMemCopia(char* v)
{int i, n;
char *p;
if(v==0) R 0;
n=strlen(v);
if(n<=0) R 0;
p=(char*) malloc(n+1);
if(p==0) R 0;
for(i=0; i<n; ++i)
p[i]=v[i];
p[i]=0;
R p;
}

class T{
public:
char *num_, *surname_;
char *key_;

What is num for? What does it represent?

Then you confuse key and surname or totally misuse the terms.

T(){num_=0; surname_=0; key_=0;}

int alloca(char *num, char *key, char *surname)
{num_=0; surname_=0; key_=0;
num_=faiMemCopia(num);
if(num_==0) return 0;
surname_=faiMemCopia(surname);
if(surname==0)
      {la0:
       free(num_); num_=0;
       R 0;
      }
key_=faiMemCopia(key);
if(key_==0){free(surname_); surname_=0;
             goto la0;
            }
R 1;
}

void Tfree(void)
{free(num_); free(key_); free(surname_);
num_=0; surname_=0; key_=0;
}

T& operator=(T& r)
{free(num_); free(key_); free(surname_);
alloca(r.num_, r.key_, r.surname_);
R *this;
}

};

class ArrArrT{
public:
T** v;
int n;
int sz;

ArrArrT(){v=0; n=0; sz=0;}

int add(char *num, char *key, char *surname)
{if(sz<=n){T **p;
            p=(T**)realloc(v, (n+128)*sizeof(T*));
            if(p==0) R 0;
            sz=n+128;
            v =p;
           }
  v[n]=(T*) malloc(sizeof(T));
  if(v[n]==0) R 0;
  if( v[n]->alloca(num, key, surname)==0)
               {free(v[n]); R 0;}
  ++n;
  R 1;
}

void sort()
{int i, hit=1, len=n;
T *temp;

while(len>1&&hit)
    {len--;
     hit=0;
     for(i=0; i<len; ++i)
       if(strcmp(v[i]->key_,v[i+1]->key_)>0)
           {temp=v[i]; v[i]=v[i+1]; v[i+1]=temp; hit=1;}
    }
}

~ArrArrT(){int i=n;
           for(--i; i>=0; --i)
              {v[i]->Tfree();
               free(v[i]);
              }
           free(v);
          }

T& operator[](int i)
{static T no;
if(i<0||i>=n)
   {cout << "\n\aIndice fuori dei limiti\n"; R no;}
R *(v[i]);
}

};

int main(void)
{int i;
ArrArrT a;
i=1;
i*=a.add("a","Mann","Thomas");

Your interface says: add(num, key, surname). Why are you using
it as a add(whatever, surname, firstname) ?

i*=a.add("b","Satie","Erik");
i*=a.add("c","Goldfarb","Sarah");
i*=a.add("d","Ravel","Maurice");
i*=a.add("e","Hideyuki","Tanaka");
i*=a.add("f","Twain","Mark");
if(i==0) {cout << "Memory error\n"; R 0;}
a.sort();

for(i=0; i<a.n; ++i)
cout <<a.v[i]->surname_<<"\t"<<a.v[i]->key_<<"\n";

for(i=0; i<a.n; ++i)
cout <<a[i].surname_<<"\t"<<a[i].key_<<"\n";

R 0;
}

Sarah Goldfarb

So a[i].surname_ print the first name while a[i].key_ prints the
surname. Very nice and intuitive... NOT!

Use C++, the STL is your friend:

#include <vector>
#include <algorithm>
#include <string>
#include <iostream>

struct Person
{
  std::string surname_;
  std::string firstname_;
  Person(std::string const & firstname, std::string const & surname):
    surname_(surname), firstname_(firstname_(firstname) {};
};

bool operator<(Person const & lhs, Person const & rhs)
{
  if(lhs.surname_ == rhs.surname_)
    return lhs.firstname_ < rhs.firstname_;
  else
    return lhs.surname_ < rhs.surname_;
}

int main()
{
  std::vector<Person> v;
  v.push_back(Person("Thomas", "Mann"));
  v.push_back(Person("Erik", "Satie"));
  v.push_back(Person("Sarah", "Goldfarb"));
  v.push_back(Person("Maurice", "Ravel"));
  v.push_back(Person("Tanaka", "Hideyuki"));
  v..push_back(Person("Mark", "Twain"));
  v.push_back(Person("Zara", "Twain"));

  std::sort(v.begin(), v.end());

  for(std::vector<Person>::const_iterator it = v.begin();
      it != v.end(); ++it )
  {
    std::cout << it->firstname_ << "\t" << it->surnamname_
    << std::endl;
  }
  return 0;
}

The big difference between this code and yours is that I don't need to
debug any of the bytes of this code :-)