I'm a newbie. Is this code ugly?
gert
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...
class Sortable
{
public:
char *key;
Sortable *temp;
Sortable ** sink(int len, Sortable **s)
{
int hit=1;
while(len>1&&hit){
len--;
hit=0;
for(int n=0; n<len; n++)
if(strcmp(s[n]->key, s[n+1]->key)>0){
temp=s[n];
s[n]=s[n+1];
s[n+1]=temp;
hit=1;
}
}
return s;
}
Sortable(){
}
};
class T: public Sortable
{
public:
char *num, *surname;
T(char *num, char *key, char *surname)
{
this->key=key;
this->num=num;
this->surname=surname;
}
};
main()
{
T a("a","Mann","Thomas");
T b("b","Satie","Erik");
T c("c","Goldfarb","Sarah");
T d("d","Ravel","Maurice");
T e("e","Hideyuki","Tanaka");
T f("f","Twain","Mark");
T *array[]= {&a, &b, &c, &d, &e, &f};
a.sink(6, (Sortable **) array);
for(int i=0; i<6; i++)
cout <<array[i]->surname<<"\t"<<array[i]->key<<"\n";
}
Andrew Poelstra
> 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...
>
Well, the short answer is yes, this is pretty ugly. Are you
coming from C or a complete newbie? If it is the latter you
should probably find a better textbook (or teacher).
> class Sortable
> {
> public:
> char *key;
Look into std::string, rather than using char * to store
text. Also, don't use tabs on Usenet; even assuming that
they can make it through all the necessary newsservers
(a big if), they normally display as 8 spaces, which is
far too much on such a space-constrained medium. Try using
2 or 4 space tabs instead.
> Sortable *temp;
This is unnecessary, and /certainly/ should not be public
even if it was necessary.
> Sortable ** sink(int len, Sortable **s)
Instead of using a pointer-to-pointer, which is almost never
necessary in C++ (this is not C!), try using a std::list or
similar container. Your code will be cleaner and you will
avoid the double indirection, the first parameter, and all
the array boundary-crossing risks.
> {
If you're going to declare a temp object, do it in here.
> int hit=1;
> while(len>1&&hit){
> len--;
> hit=0;
> for(int n=0; n<len; n++)
> if(strcmp(s[n]->key, s[n+1]->key)>0){
> temp=s[n];
> s[n]=s[n+1];
> s[n+1]=temp;
> hit=1;
> }
> }
I strongly recommend you find a book on sorting algorithms, or
post this code on comp.programming for feedback. This looks like
a bubble sort, which does not scale well.
> return s;
> }
> Sortable(){
> }
There is no need for an empty constructor. The compiler will
provide one for you.
> };
> class T: public Sortable
> {
> public:
> char *num, *surname;
> T(char *num, char *key, char *surname)
> {
> this->key=key;
> this->num=num;
> this->surname=surname;
> }
> };
> main()
int main() or int main(void), though I have been told the latter is
a C-ism and not really appropriate in C++. In any case, main() returns
int.
> {
> T a("a","Mann","Thomas");
> T b("b","Satie","Erik");
> T c("c","Goldfarb","Sarah");
> T d("d","Ravel","Maurice");
> T e("e","Hideyuki","Tanaka");
> T f("f","Twain","Mark");
> T *array[]= {&a, &b, &c, &d, &e, &f};
> a.sink(6, (Sortable **) array);
> for(int i=0; i<6; i++)
> cout <<array[i]->surname<<"\t"<<array[i]->key<<"\n";
> }
What I have told you is an incomplete first pass over your code. If you
look into the things I suggested and check out the C++ FAQ, you will see
a lot of improvement.
io_x
"gert" <27h...@googlemail.com> ha scritto nel messaggio
news:e4951ec7-f09b-4b74...@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?
#include <iostream>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
using namespace std;
class T{
public:
char *num_, *surname_;
char *key_;
};
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) return 0;
sz=n+128;
v =p;
}
v[n]=(T*) malloc(sizeof(T));
if(v[n]==0) return 0;
v[n]->num_=num; v[n]->key_=key; v[n]->surname_=surname;
++n;
return 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)
free(v[i]);
free(v);
}
};
int main(void)
{int i;
ArrArrT a;
i=1;
i*=a.add("a","Mann","Thomas");
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"; return 0;}
a.sort();
for(i=0; i<a.n; ++i)
cout <<a.v[i]->surname_<<"\t"<<a.v[i]->key_<<"\n";
return 0;
}
Richard Herring
<a...@b.c.invalid> writes
>
>"gert" <27h...@googlemail.com> ha scritto nel messaggio
>news:e4951ec7-f09b-4b74...@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.
>#include <iostream>
>#include <stdio.h>
>#include <stdlib.h>
>#include <ctype.h>
>using namespace std;
>
>class T{
>public:
> char *num_, *surname_;
> char *key_;
C++ isn't C -- use std::string in preference to dynamic arrays.
>};
>
>class ArrArrT{
>public:
>T** v;
C++ isn't C -- use std::vector in preference to dynamic arrays.
>int n;
>int sz;
>
> ArrArrT(){v=0; n=0; sz=0;}
>
> int add(char *num, char *key, char *surname)
This function returns a boolean value, so don't pretend it's a number.
(Since the "failure" condition actually indicates that malloc failed,
and you should be delegating that kind of memory management to
std::vector, it would be better to make the function void and leave it
to vector to throw an exception if it can't allocate.)
> {if(sz<=n){T **p;
> p=(T**)realloc(v, (n+128)*sizeof(T*));
C++ isn't C -- use std::vector in preference to malloc and friends.
What's the magic number supposed to be for?
> if(p==0) return 0;
> sz=n+128;
> v =p;
> }
> v[n]=(T*) malloc(sizeof(T));
> if(v[n]==0) return 0;
> v[n]->num_=num; v[n]->key_=key; v[n]->surname_=surname;
> ++n;
> return 1;
> }
>
>void sort()
>{int i, hit=1, len=n;
hit appears to be a Boolean flag, not a number -- declare it as such.
> T *temp;
C++ isn't C. Declare local variables at the point of use -- if you need
them at all.
Someone else can comment on this O(N^2) code from an algorithmic POV.
> 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;}
Use std::swap for swapping.
> }
>}
>
> ~ArrArrT(){int i=n;
> for(--i; i>=0; --i)
> free(v[i]);
> free(v);
> }
>};
>
>int main(void)
>{int i;
Don't separate declaration and initialization.
> ArrArrT a;
> i=1;
> i*=a.add("a","Mann","Thomas");
> 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");
This code works, after a fashion, because those strings are all
literals. What would happen if you were reading values from a file?
> if(i==0) {cout << "Memory error\n"; return 0;}
> a.sort();
>
> for(i=0; i<a.n; ++i)
> cout <<a.v[i]->surname_<<"\t"<<a.v[i]->key_<<"\n";
> return 0;
>}
>
>
>
--
Richard Herring
io_x
"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
news:4iB21VKQ...@baesystems.com...
> In message <4b56d0c2$0$828$4faf...@reader5.news.tin.it>, io_x <a...@b.c.invalid>
> writes
>>
>>"gert" <27h...@googlemail.com> ha scritto nel messaggio
>>news:e4951ec7-f09b-4b74...@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.
> 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
#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_;
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;
}
};
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)
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");
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
Tanaka Hideyuki
Thomas Mann
Maurice Ravel
Erik Satie
Mark Twain
Sarah Goldfarb
Tanaka Hideyuki
Thomas Mann
Maurice Ravel
Erik Satie
Mark Twain
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio
news:4b57269b$0$1132$4faf...@reader1.news.tin.it...
i think here there is one error should be
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]);
> }
this has some problem can be resolved in a custom operator= ??
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio
news:4b57269b$0$1132$4faf...@reader1.news.tin.it...
>
> "Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
> news:4iB21VKQ...@baesystems.com...
>> In message <4b56d0c2$0$828$4faf...@reader5.news.tin.it>, io_x
>> <a...@b.c.invalid>
>> writes
>>>
>>>"gert" <27h...@googlemail.com> ha scritto nel messaggio
>>>news:e4951ec7-f09b-4b74...@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.
but i can debug all single bit of that
>> 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
T(){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;}
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio news:...
> T& operator=(T& r)
> {free(num_); free(key_); free(surname_);
> alloca(r.num_, r.key_, r.surname_);
> R *this;
> }
i forget the case of &r==this
T& operator=(T& r)
{if(&r!=this)
Richard Herring
<a...@b.c.invalid> trolled
>
>"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
>news:4iB21VKQ...@baesystems.com...
>> In message <4b56d0c2$0$828$4faf...@reader5.news.tin.it>, io_x
>><a...@b.c.invalid>
>> writes
>>>
>>>"gert" <27h...@googlemail.com> ha scritto nel messaggio
>>>news:e4951ec7-f09b-4b74...@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.
>
>> 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
Still horrible.
>
>#include <iostream>
>#include <stdio.h>
>#include <stdlib.h>
>#include <ctype.h>
>#define R return
Using macros to represent language keywords is inexcusable obfuscation.
Are you J*ff R*lf?
>using namespace std;
>
>char* faiMemCopia(char* v)
Why are you (badly) reinventing strcpy() ?
>{int i, n;
> char *p;
> if(v==0) R 0;
> n=strlen(v);
> if(n<=0) R 0;
Why? There's nothing intrinsically wrong with a zero-length string.
> 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_;
>
> T(){num_=0; surname_=0; key_=0;}
>
> int alloca(char *num, char *key, char *surname)
This sets up the class invariant. Put it in a constructor.
> {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;
Nice spaghetti.
> }
> R 1;
> }
>
> void Tfree(void)
> {free(num_); free(key_); free(surname_);
> num_=0; surname_=0; key_=0;
> }
This releases resources. Put it in a destructor.
>
>};
>
>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;
Still no explanation for those magic 128s.
Sheesh.
#include <string>
#include <vector>
#include <algorithm>
#include <ostream>
#include <iostream>
#include <iterator>
using namespace std; // SSM
struct Artist
{
string surname_, firstname_;
Artist(string const & surname, string const & firstname)
: surname_(surname), firstname_(firstname) {}
};
ostream & operator<<(ostream & s, Artist const & t)
{ return s << t.firstname_ << ' ' << t.surname_; }
struct CompareSurname
{
bool operator()(Artist const & a, Artist const & b) const
{ return a.surname_ < b.surname_; }
};
int main()
{
vector<Artist> artists;
artists.push_back(Artist("Mann", "Thomas"));
artists.push_back(Artist("Satie", "Erik"));
/* etc... */
sort(artists.begin(), artists.end(), CompareSurname());
copy(artists.begin(), artists.end(), ostream_iterator<Artist>(cout, "\n"));
}
--
Richard Herring
Yannick Tremblay
io_x <a...@b.c.invalid> wrote:
>
>"io_x" <a...@b.c.invalid> ha scritto nel messaggio
>news:4b57269b$0$1132$4faf...@reader1.news.tin.it...
>>
>> "Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
>> news:4iB21VKQ...@baesystems.com...
>>> In message <4b56d0c2$0$828$4faf...@reader5.news.tin.it>, io_x
>>> <a...@b.c.invalid>
>>> writes
>>>>
>>>>"gert" <27h...@googlemail.com> ha scritto nel messaggio
>>>>news:e4951ec7-f09b-4b74...@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.
What is num for? What does it represent?
Then you confuse key and surname or totally misuse the terms.
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 :-)
gert
Sorry for mixing up forename and surname.
>I strongly recommend you find a book on sorting algorithms, or
>post this code on comp.programming for feedback. This looks like
>a bubble sort, which does not scale well.
Yes, sinking sort is just an other name for bubble sort ;)
>Instead of using a pointer-to-pointer, which is almost never
>necessary in C++ (this is not C!), try using a std::list or
>similar container. Your code will be cleaner and you will
>avoid the double indirection, the first parameter, and all
>the array boundary-crossing risks.
Yes, I'm coming from C.
I also admit the example is a bit stupid. Who would ever need a class
to sort arrays of instances of itself?
But anyway, this was an exercise given to me and I am trying to make
the best of it.
We're not allowed to use the STL. And it has to be an array of
objects, no structs, no vektors.
So is this justification for using a pointer-to-pointer? Or would
there still a better alternative?
>> Sortable *temp;
>This is unnecessary
Unnecessary? I should've put it further down like you said later on,
but you need a temp somewhere don't you?
Andrew Poelstra
> Thank you guys. I also love io_x's artwork :D
> Sorry for mixing up forename and surname.
>
You dropped my name! But I forgive you. :)
>>I strongly recommend you find a book on sorting algorithms, or
>>post this code on comp.programming for feedback. This looks like
>>a bubble sort, which does not scale well.
>
> Yes, sinking sort is just an other name for bubble sort ;)
>
Ah. Given that it has almost zero overhead it actually works
quite well for sorting a few dozen items, but when you go
beyond that the O(n^2) will catch up with you.
>>Instead of using a pointer-to-pointer, which is almost never
>>necessary in C++ (this is not C!), try using a std::list or
>>similar container. Your code will be cleaner and you will
>>avoid the double indirection, the first parameter, and all
>>the array boundary-crossing risks.
>
> Yes, I'm coming from C.
So am I - I'm actually far more a C programmer than a C++ programmer.
One of the most important lessons you can learn is that C and C++,
while sharing a lot of the same keywords and 1/3 of the name, are
very different languages, and in general require very different
mindsets to work with.
> I also admit the example is a bit stupid. Who would ever need a class
> to sort arrays of instances of itself?
> But anyway, this was an exercise given to me and I am trying to make
> the best of it.
> We're not allowed to use the STL. And it has to be an array of
> objects, no structs, no vektors.
Fair enough. But I think that C++ is perhaps a poor choice of language
for this low-level programming practice.
> So is this justification for using a pointer-to-pointer? Or would
> there still a better alternative?
>
Yep. In function calls you can use the suffix [] instead of the
prefix *. They mean the same thing but the former makes it clear
that you want a pointer to an array, not just a single object.
As for your second level of indirection, consider using pass by
reference instead.
>>> Sortable *temp;
>>This is unnecessary
>
> Unnecessary? I should've put it further down like you said later on,
> but you need a temp somewhere don't you?
Yes. I should have been clearer. Though, a very common exercise
in beginning C++ is to write a simple swap function, first to
learn about pass-by-reference, and then later to learn about
templates. (And as has been mentioned, there is a std::swap()
function available for you to use.)
io_x
"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
yes here i would say "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_;
>>
>> T(){num_=0; surname_=0; key_=0;}
>>
>> int alloca(char *num, char *key, char *surname)
>
> This sets up the class invariant. Put it in a constructor.
here i have one array of T* ;
the problem is always the same:
i have a class Y
Y *p;
p is class pointer
than i want build for p the Y class
than i don't know how use constructor-destructor for Y for that object
what i can do is
p=(Y*) malloc(sizeof(Y));
p->inizialize();
and
for deallocate it
p->deinizialize();
free(p);
in the few i understand i can use constructor-destructor
if the language allow that for use with the type Y* (other than Y)
it realloc the array of poiters always more,
[it is possible write code for reduce it too]
for me 128 is a nice number
io_x
"Andrew Poelstra" <apoe...@localhost.localdomain> ha scritto nel messaggio
news:slrnhlfkv5.j...@localhost.localdomain...
> On 2010-01-21, gert <27h...@googlemail.com> wrote:
>> Thank you guys. I also love io_x's artwork :D
> in beginning C++ is to write a simple swap function, first to
> learn about pass-by-reference, and then later to learn about
> templates.
do you mean something like
void swap(T&, T&);
the swap exercise you refer is one not good use of reference &:
if to be good it has to be
void swap(T*, T*)
using in swap(&v, &w)
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio
news:4b57ff58$0$1135$4faf...@reader1.news.tin.it...
what about use of the template?
yes i write horrible code, but not all the sheepe are white;
Is it possible use the assembly language for write template functions?
i think yes it is enought to pass the sizeof(T) like argument
and all should goes well
if yes template could be all good
how i can use cpm1 in a.sort() function?
do you think there are memory leak? (i don't have here my functions)
thank you
#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 Person{
public:
char *num_, *surname_;
char *key_;
Person(){num_=0; surname_=0; key_=0;}
int InitializePointer(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;
}
Person(char *num, char *key, char *surname)
{if( this->InitializePointer(num, key, surname) ==0)
{cerr << "Need more memory\n"; exit(0);}
}
Person(Person& a)
{if( this->InitializePointer(a.num_, a.key_, a.surname_) ==0)
{cerr << "Need more memory\n"; exit(0);}
}
void FreePointer(void)
{free(num_); free(key_); free(surname_);
num_=0; surname_=0; key_=0;
}
~Person(){this->FreePointer();}
Person& operator=(Person& r)
{if(&r!=this)
{free(num_); free(key_); free(surname_);
if(this->InitializePointer(r.num_, r.key_, r.surname_)==0)
{cerr << "Need more memory\n"; exit(0);}
}
R *this;
}
int cmp1(Person& a, Person& b){R strcmp(a.key_,b.key_);}
};
template <class T> class RosList{
public:
T** v;
int n;
int sz;
RosList(){v=0; n=0; sz=0;}
int add(T& a)
{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;
*(v[n])=a; ++n;
R 1;
}
void sort(int (*cmp)(T& a, T& b))
{int i, hit=1, len=n;
T *temp;
while(len>1&&hit)
{len--;
hit=0;
for(i=0; i<len; ++i)
if(cmp( *(v[i]), *(v[i+1]) )>0)
{temp=v[i]; v[i]=v[i+1]; v[i+1]=temp; hit=1;}
}
}
~RosList(){int i=n;
for(--i; i>=0; --i)
{v[i]->FreePointer();
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 cmp(Person& a, Person& b){R strcmp(a.key_,b.key_);}
int main(void)
{int i;
RosList<Person> a;
i=1;
i*=a.add(Person("a","Mann","Thomas"));
i*=a.add(Person("b","Satie","Erik"));
i*=a.add(Person("c","Goldfarb","Sarah"));
i*=a.add(Person("d","Ravel","Maurice"));
i*=a.add(Person("e","Hideyuki","Tanaka"));
i*=a.add(Person("f","Twain","Mark"));
if(i==0) {cout << "Memory error\n"; R 0;}
a.sort(cmp);
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio
> "io_x" <a...@b.c.invalid> ha scritto nel messaggio
> news:4b57ff58$0$1135$4faf...@reader1.news.tin.it...
>
> what about use of the template?
> yes i write horrible code, but not all the sheepe are white;
>
> Is it possible use the assembly language for write template functions?
> i think yes it is enought to pass the sizeof(T) like argument
> and all should goes well
>
> if yes template could be all good
>
> how i can use cpm1 in a.sort() function?
> do you think there are memory leak? (i don't have here my functions)
> thank you
> {if(&r!=this)
> {free(num_); free(key_); free(surname_);
> if(this->InitializePointer(r.num_, r.key_, r.surname_)==0)
> {cerr << "Need more memory\n"; exit(0);}
> }
> R *this;
> }
>
> public:
> T** v;
> int n;
> int sz;
>
> RosList(){v=0; n=0; sz=0;}
>
> int add(T& a)
> {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;
> *(v[n])=a; ++n;
here there is a problem because
operator=(Person& r)
has free(num_); free(key_); free(surname_);
with num_ key_ surname_ !=0
-------
#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 Person{
public:
char *num_, *surname_;
char *key_;
Person(){num_=0; surname_=0; key_=0;}
void InitializePointer(char *num, char *key, char *surname)
{num_=0; surname_=0; key_=0;
num_=faiMemCopia(num);
if(num_==0) {la:
cerr << "Need more memory\n";
exit(0);
}
surname_=faiMemCopia(surname);
if(surname==0)
{la0:
free(num_); num_=0;
goto la;
}
key_=faiMemCopia(key);
if(key_==0){free(surname_); surname_=0;
goto la0;
}
}
void InitializePointer(Person& a)
{InitializePointer(a.num_, a.key_, a.surname_);}
Person(char *num, char *key, char *surname)
{this->InitializePointer(num, key, surname);}
Person(Person& a){this->InitializePointer(a);}
void FreePointer(void)
{free(num_); free(key_); free(surname_);
num_=0; surname_=0; key_=0;
}
~Person(){this->FreePointer();}
Person& operator=(Person& r)
{if(&r!=this)
{free(num_); free(key_); free(surname_);
this->InitializePointer(r);
}
R *this;
}
int cmp1(Person& a, Person& b){R strcmp(a.key_,b.key_);}
};
template <class T> class RosList{
public:
T** v;
int n;
int sz;
RosList(){v=0; n=0; sz=0;}
int add(T& a)
{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;
v[n]->InitializePointer(a);
Fred Zwarts
> "Andrew Poelstra" <apoe...@localhost.localdomain> ha scritto nel
> messaggio news:slrnhlfkv5.j...@localhost.localdomain...
>> On 2010-01-21, gert <27h...@googlemail.com> wrote:
>>> Thank you guys. I also love io_x's artwork :D
>> Yes. I should have been clearer. Though, a very common exercise
>> in beginning C++ is to write a simple swap function, first to
>> learn about pass-by-reference, and then later to learn about
>> templates.
>
> do you mean something like
> void swap(T&, T&);
> the swap exercise you refer is one not good use of reference &:
Why not?
> if to be good it has to be
> void swap(T*, T*)
> using in swap(&v, &w)
Be careful when using pointers instead of references!
With pointers a check for NULL is needed.
So, I would prefer "void swap (T&, T&)",
not void swap "(T*, T*)", because of simpler code.
Richard Herring
<a...@b.c.invalid> writes
And under what circumstances would strlen() ever return a negative
value?
[...]
>>>
>>> int alloca(char *num, char *key, char *surname)
>>
>> This sets up the class invariant. Put it in a constructor.
>
>here i have one array of T* ;
>the problem is always the same:
>i have a class Y
>Y *p;
>p is class pointer
>than i want build for p the Y class
>than i don't know how use constructor-destructor for Y for that object
>
>what i can do is
>p=(Y*) malloc(sizeof(Y));
>p->inizialize();
>and
>for deallocate it
>p->deinizialize();
>free(p);
The C++ way to do that is:
p = new Y;
/* ... */
delete p;
and the constructor and destructor of Y will be called without any work
on your part.
--
Richard Herring
gert
>
> p = new Y;
> /* ... */
>
> delete p;
So how about Y p(); delepte p; Is this still C++ way?
Richard Herring
<346df13f-03e4-4e43...@e27g2000yqd.googlegroups.com>,
gert <27h...@googlemail.com> writes
>> The C++ way to do that is:
>>
>> p = new Y;
>> /* ... */
>>
>> delete p;
>
>So how about Y p();
(I don't think you meant that. It declares a function called p which
takes no arguments and returns a Y. Try just:
Y p;
;-)
>delepte p; Is this still C++ way?
No. If you don't use new, you don't use delete, either.
This is the C++ way:
{
Y p; // here the constructor of Y is called
} // here the destructor of Y is called as p goes out of scope.
--
Richard Herring
LR
>>> p=(T**)realloc(v, (n+128)*sizeof(T*));
>>> if(p==0) R 0;
>>> sz=n+128;
>>
>> Still no explanation for those magic 128s.
> for me 128 is a nice number
How would anyone reading your code know that 128 is a nice number?
How would know, and if they could, would they care, that 128 is a nice
number, for you?
Are those two uses of 128 related in some way? If so, how would someone
reading the code know that?
LR
LR
> "io_x" <a...@b.c.invalid> ha scritto nel messaggio
> news:4b57ff58$0$1135$4faf...@reader1.news.tin.it...
> yes i write horrible code, but not all the sheepe are white;
I completely fail to understand your point. Please explain this.
LR
io_x
"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
it seems strlen() here returned one unsigned type: size_t
here size_t is "unsigned int" of 32 bit but this means
strlen can return 0xF0000000 that is a negative number
seen it like int; and for me that negative number is not ok
> [...]
>
>>>>
>>>> int alloca(char *num, char *key, char *surname)
>>>
>>> This sets up the class invariant. Put it in a constructor.
>>
>>here i have one array of T* ;
>>the problem is always the same:
>>i have a class Y
>>Y *p;
>>p is class pointer
>>than i want build for p the Y class
>>than i don't know how use constructor-destructor for Y for that object
>>
>>what i can do is
>>p=(Y*) malloc(sizeof(Y));
>>p->inizialize();
>>and
>>for deallocate it
>>p->deinizialize();
>>free(p);
>
> The C++ way to do that is:
>
> p = new Y;
> /* ... */
>
> delete p;
>
> and the constructor and destructor of Y will be called without any work on
> your part.
thank you, i did not think about that, thanks
why the default operator delete
has one argument of type size_t?
void operator delete(void*, size_t);
there is something hidden in the call delete?
for example here
T a(1,1,1);
T *p=new T(a)
...
delete p;
-----------------------------
#include <iostream>
#include <cstdio>
#include <cstdlib>
#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 Person{
public:
char *num_, *surname_;
char *key_;
Person(){num_=0; surname_=0; key_=0;}
void InitializePointer(char *num, char *key, char *surname)
{num_=0; surname_=0; key_=0;
num_=faiMemCopia(num);
if(num_==0) {la:
cerr << "Need more memory\n";
exit(0);
}
surname_=faiMemCopia(surname);
if(surname==0)
{la0:
free(num_); num_=0;
goto la;
}
key_=faiMemCopia(key);
if(key_==0){free(surname_); surname_=0;
goto la0;
}
}
void InitializePointer(Person& a)
{InitializePointer(a.num_, a.key_, a.surname_);}
Person(char *num, char *key, char *surname)
{this->InitializePointer(num, key, surname);}
Person(Person& a){this->InitializePointer(a);}
void FreePointer(void)
{free(num_); free(key_); free(surname_);
num_=0; surname_=0; key_=0;
}
~Person(){this->FreePointer();}
Person& operator=(Person& r)
{if(&r!=this)
{free(num_); free(key_); free(surname_);
this->InitializePointer(r);
}
R *this;
}
int cmp1(Person& a, Person& b){R strcmp(a.key_,b.key_);}
};
template <class T> class RosList{
public:
T** v;
int n;
int sz;
// these function are not called
void* operator new(size_t sz){R malloc(sz);}
void operator delete(void* p){free(p);}
RosList(){v=0; n=0; sz=0;}
int add(T& a)
{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] = new T(a);
if(v[n]==0) R 0;
++n;
R 1;
}
void sort(int (*cmp)(T& a, T& b))
{int i, hit=1, len=n;
T *temp;
while(len>1&&hit)
{len--;
hit=0;
for(i=0; i<len; ++i)
if(cmp( *(v[i]), *(v[i+1]) )>0)
{temp=v[i]; v[i]=v[i+1]; v[i+1]=temp; hit=1;}
}
}
int sort(int i1, int i2, int (*cmp)(T& a, T& b))
{int i, hit=1, len;
T *temp;
if(i1==i2&&i2<n&&i2>=0) R 1;
if(i1>=i2) R 0;
if(i2> n) R 0;
len=i2-i1;
while(len>1&&hit)
{len--;
hit=0;
for(i=i1; i<len; ++i)
if(cmp( *(v[i]), *(v[i+1]) )>0)
{temp=v[i]; v[i]=v[i+1]; v[i+1]=temp; hit=1;}
}
R 1;
}
~RosList(){int i=n;
for(--i; i>=0; --i)
delete 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 cmp(Person& a, Person& b){R strcmp(a.key_,b.key_);}
int cmp2(Person& a, Person& b){R strcmp(a.num_,b.num_);}
int main(void)
{int i;
RosList<Person> a;
i=1;
i*=a.add(Person("a","Mann","Thomas"));
i*=a.add(Person("b","Satie","Erik"));
i*=a.add(Person("c","Goldfarb","Sarah"));
i*=a.add(Person("d","Ravel","Maurice"));
i*=a.add(Person("e","Hideyuki","Tanaka"));
i*=a.add(Person("f","Twain","Mark"));
if(i==0) {cout << "Memory error\n"; R 0;}
a.sort(cmp);
for(i=0; i<a.n; ++i)
cout <<a.v[i]->surname_<<"\t"<<a.v[i]->key_<<"\n";
cout << "------------------------\n";
a.sort(0, a.n, cmp2);
for(i=0; i<a.n; ++i)
cout <<a[i].surname_<<"\t"<<a[i].key_<<"\n";
R 0;
}
-------------
Sarah Goldfarb
Tanaka Hideyuki
Thomas Mann
Maurice Ravel
Erik Satie
Mark Twain
------------------------
Thomas Mann
Erik Satie
Sarah Goldfarb
Maurice Ravel
Tanaka Hideyuki
Mark Twain
Richard Herring
>
>"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
>news:FLTpfxBU...@baesystems.com...
>> In message <4b57ff58$0$1135$4faf...@reader1.news.tin.it>, io_x
>> <a...@b.c.invalid> writes
>>>
>>>>>
>>>>
>>>> Why are you (badly) reinventing strcpy() ?
>>>>
>>>>>{int i, n;
>>>>> char *p;
>>>>> if(v==0) R 0;
>>>>> n=strlen(v);
>>>>> if(n<=0) R 0;
>>>>
>>>> Why? There's nothing intrinsically wrong with a zero-length string.
>>>
>>>yes here i would say "if(n<0) R 0;"
>>
>> And under what circumstances would strlen() ever return a negative value?
>
>it seems strlen() here returned one unsigned type: size_t
>here size_t is "unsigned int" of 32 bit but this means
>strlen can return 0xF0000000 that is a negative number
>seen it like int;
Only because you are perversely choosing to cast it to a signed type
when it is not.
>and for me that negative number is not ok
So you should be declaring the variable n as size_t, not int.
[...]
>>>what i can do is
>>>p=(Y*) malloc(sizeof(Y));
>>>p->inizialize();
>>>and
>>>for deallocate it
>>>p->deinizialize();
>>>free(p);
>>
>> The C++ way to do that is:
>>
>> p = new Y;
>> /* ... */
>>
>> delete p;
>>
>> and the constructor and destructor of Y will be called without any work on
>> your part.
>
>thank you, i did not think about that, thanks
>
>why the default operator delete
>has one argument of type size_t?
It doesn't. A class-specific operator delete may have a second argument
of type size_t.
>
>void operator delete(void*, size_t);
>
>there is something hidden in the call delete?
Don't confuse the delete expression "delete p;" with the deallocation
function named "operator delete()". They do different things.
>for example here
>T a(1,1,1);
>T *p=new T(a)
Why the unnecessary copy? Just write:
T * p = new T(1,1,1);
>...
>delete p;
[snip horrid C-style code]
--
Richard Herring
Jerry Coffin
22g2000yqr.googlegroups.com>, 27h...@googlemail.com says...
>
> 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...
Without passing judgment about "ugly", I'd say the design and code
are quite a bit different from what you'd expect in well-written,
idiomatic C++. To accomplish the same general task, I'd typically
write something more like this:
class T {
// the num member is read in, but never used...
std::string key_, num_, surname_;
public:
T(std::string num, std::string key, std::string surname)
: key_(key), num_(num), surname_(surname)
{}
bool operator<(T const &other) const {
return key_ < other.key_;
}
friend std::ostream &operator<<(std::ostream &os, T const &t) {
return os << t.surname << "\t" << t.key;
};
int main() {
std::vector<T> t;
t.push_back(T("a","Mann","Thomas"));
t.push_back(T("b","Satie","Erik"));
t.push_back(T("c","Goldfarb","Sarah"));
t.push_back(T("d","Ravel","Maurice"));
t.push_back(T("e","Hideyuki","Tanaka"));
t.push_back(T("f","Twain","Mark"));
std::sort(t.begin(), t.end());
std::copy(t.begin(), t.end(),
std::ostream_iterator<T>(std::cout, "\n"));
return 0;
}
Of course, there are quite a few variations, but I hope this at least
gives some general idea.
--
Later,
Jerry.
io_x
"LR" <lr...@superlink.net> ha scritto nel messaggio
news:4b58f392$0$4817$cc2e...@news.uslec.net...
the sheep white as programmer write all good code follow what
people say are the good laws for write code
the sheep black as programmer not
> LR
>
>
io_x
"LR" <lr...@superlink.net> ha scritto nel messaggio
news:4b58f327$0$4817$cc2e...@news.uslec.net...
yes, this could be the wrong number,
if for example someone has 1000000s of little double dinamic arrays
> LR
LR
So it's possible that someone, perhaps even you might want to change
that instance of 128 in future?
Perhaps you ought to do something like:
static const size_increase = 128;
p=(T**)realloc(v, (n+size_increase)*sizeof(T*));
if(p==0) return 0; // I think R 0; is harder to read.
sz=n+size_increase;
Which will also save you from making the mistake of not hunting down the
second related occurrence.
LR
LR
It seems to me, after reading this group for sometime, that most of the
contributors are more interested in following what more people think of
as being good rules for writing code.
The reasons for this are varied and there is always much debate.
There seems to be one area where there is substantial agreement. Code
is not only (or merely or simply) a means to instruct a computer to
follow some instructions, but a means to communicate ideas to other
people clearly.
The reasons for this are varied and there is always much debate.
However, I think, since you are interested in finding out if the code
was ugly, you may want to, for starters, ask if the way the code is
written aids clarity or hinders it.
I am otherwise at a loss as to why the question in the subject was asked
to begin with. Particularly since you imply that you are not one of the
people who wants to follow good rules. Doesn't this suggest that the
answer to your question obvious? And why would you ask here?
There are people who are interested in not following good rules and in
particular avoiding clarity. You may find much of value to you here
http://en.wikipedia.org/wiki/Obfuscated_code
LR
io_x
"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
>>>>>> char *p;
>>>>>> if(v==0) R 0;
>>>>>> n=strlen(v);
>>>>>> if(n<=0) R 0;
>>>>>
>>>>> Why? There's nothing intrinsically wrong with a zero-length string.
>>>>
>>>>yes here i would say "if(n<0) R 0;"
>>>
>>> And under what circumstances would strlen() ever return a negative value?
>>
>>it seems strlen() here returned one unsigned type: size_t
>>here size_t is "unsigned int" of 32 bit but this means
>>strlen can return 0xF0000000 that is a negative number
>>seen it like int;
>
> Only because you are perversely choosing to cast it to a signed type when it
> is not.
>>and for me that negative number is not ok
>
> So you should be declaring the variable n as size_t, not int.
i choose int
so i choose that the valid array of chars
could have len in the range 0..max_int
at last here in my sys
io_x
"io_x" <a...@b.c.invalid> ha scritto nel messaggio
news:4b594ee2$0$1115$4faf...@reader4.news.tin.it...
> int sort(int i1, int i2, int (*cmp)(T& a, T& b))
> {int i, hit=1, len;
> T *temp;
>
> if(i1==i2&&i2<n&&i2>=0) R 1;
> if(i1>=i2) R 0;
> if(i2> n) R 0;
forget "if(i1<0) R 0;"
Ian Collins
>
> #include <iostream>
> #include <stdio.h>
> #include <stdlib.h>
> #include <ctype.h>
> #define R return
This is an abomination, no sane programmer would read any further.
--
Ian Collins
io_x
> io_x wrote:
>>>> yes i write horrible code, but not all the sheepe are white;
it is horrible for you; for me seems horrible this
"copy(artists.begin(), artists.end(), ostream_iterator<Artist>(cout, "\n"));"
or could be debug that
>>> I completely fail to understand your point. Please explain this.
>>
>> the sheep white as programmer write all good code follow what
>> people say are the good laws for write code
>> the sheep black as programmer not
>
> It seems to me, after reading this group for sometime, that most of the
> contributors are more interested in following what more people think of
> as being good rules for writing code.
i don't say i don't follow good rules
yes i follow good rules, the rules that show me they are right.
for example the goto instruction: it show me it could make wonderful
structured program, best than all your whiles, fors, loops etc etc
and more understandable too (at last from me)
but it seems i found no other one believe in that
> The reasons for this are varied and there is always much debate.
>
> There seems to be one area where there is substantial agreement. Code
> is not only (or merely or simply) a means to instruct a computer to
> follow some instructions, but a means to communicate ideas to other
> people clearly.
the first people who has to understand is who write the code;
if that people not understand at full: what comunicate???
> The reasons for this are varied and there is always much debate.
>
> However, I think, since you are interested in finding out if the code
> was ugly, you may want to, for starters, ask if the way the code is
> written aids clarity or hinders it.
> I am otherwise at a loss as to why the question in the subject was asked
> to begin with. Particularly since you imply that you are not one of the
> people who wants to follow good rules.
i want to follow good rules; but my rules differ from rules of others
> Doesn't this suggest that the
> answer to your question obvious? And why would you ask here?
>
> There are people who are interested in not following good rules and in
> particular avoiding clarity. You may find much of value to you here
> http://en.wikipedia.org/wiki/Obfuscated_code
the first people who has to understand is who write the code.
I don't say i wrote something i not understand at full
and it seems goes well; it is not all easy
there are places that is not easy
> LR
io_x
>me
> However, I think, since you are interested in finding out if the code
> was ugly, you may want to, for starters, ask if the way the code is
> written aids clarity or hinders it.
it hide nothing
> I am otherwise at a loss as to why the question in the subject was asked
> to begin with. Particularly since you imply that you are not one of the
> people who wants to follow good rules.
i'm no the one that ask that question (if i remember all posts)
> Doesn't this suggest that the
> answer to your question obvious?
> And why would you ask here?
i say: i'm not the one to ask at first post in this thread:
"I'm a newbie. Is this code ugly?"
because i just know what 80% think about it (it is ugly);
and prefer to be silent. it is enought it is not ugly for me
in the end of the game i not think that the OP code is so ugly
because sort array of pointers
LR
> "LR" <lr...@superlink.net> ha scritto nel messaggio
> news:4b5b359f$0$4831$cc2e...@news.uslec.net...
> i'm no the one that ask that question (if i remember all posts)
Oops. Sorry about that.
LR
Richard Herring
<a...@b.c.invalid> writes
lengths up to DBL_MAX.
Why do you have a problem with giving n the obvious type (because it is
what strlen returns) of size_t? What do you think the length range would
be in that case?
--
Richard Herring
io_x
"Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
size_t could be the "the obvious type" but with "int" i can check
for overflow checking if there is some value negative
in some places where could be overflow or afther too
if one program has
size_t t=90;
and there is one
t+=v;
and v make overflow t; what that program has to do:
0) check and return to the caller one error result
1) abort the program, the sys, the OS
2) nothing has to happen
my answer is 0
> --
> Richard Herring
Richard Herring
So you're using wrapping as a poor substitute for checking that t is in
range. On most systems, t will reach a value which exceeds the
available memory long before it wraps to a negative value.
If range checking is important, check the range. Just as
std::vector::at() (and probably operator[] too) does, using unsigned
subscripts.
>what that program has to do:
>0) check and return to the caller one error result
>1) abort the program, the sys, the OS
>2) nothing has to happen
>
>my answer is 0
Right. But for many values of v, your solution will actually give (2).
--
Richard Herring
Christopher Dearlove
news:JZfKbybi...@baesystems.com...
> So you're using wrapping as a poor substitute for checking that t is in
> range. On most systems, t will reach a value which exceeds the available
> memory long before it wraps to a negative value.
And, while it usually happens to work, wrapping of signed integers
is actually undefined behaviour.
If you are using unsigned integers (like size_t) it's easy to check wrapping
of addition (although as noted that probably is the wrong test). If a, b, c
are such values and
c = a + b;
Then wrapping has ocurred if (and only if) c < a (or, equally well, c < b).
What I don't know how to do without using division - any solutions welcome
- is to check the overflow of a * b (unsigned again). c = a * b can overflow
but with c > a and c > b. That's not the issue here, but it's one I have
wanted
(for smaller unsigned types than size_t).
Of course with regard to the original code this is nit-picking fine points
of code that the only thing to actually do with it is bin it and start
again.
It's instructive that the poster has had to revise the code at least twice
since posting to fix bugs. The polite term is unmaintainable.