Author Topic: ResbufList class  (Read 11423 times)

0 Members and 1 Guest are viewing this topic.

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
ResbufList class
« on: March 17, 2010, 11:43:18 AM »
updated
« Last Edit: November 10, 2010, 07:11:03 AM by __declspec »

LE3

  • Guest
Re: ResbufList class
« Reply #1 on: March 17, 2010, 03:12:13 PM »
Hi Dan,

First bird eyes looks good.

Once I get the chance to implement some autolisp functions again from one of my projects, will give a full try out - you should have written this class like seven years ago :).

Is this yours? (I am asking because do not see your name on it).

Thanks!

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #2 on: March 17, 2010, 07:06:26 PM »
Hi Dan,

First bird eyes looks good.

Once I get the chance to implement some autolisp functions again from one of my projects, will give a full try out - you should have written this class like seven years ago :).

Thanks! It's not only for lisp, it should be useful when working with XData, XRecords, Selection Set filters... or where you need to use a resbuf..


Is this yours? (I am asking because do not see your name on it).

of course its not mine, its yours, that's why I wrote it (note the "DRM" on the header)  :-D
...well with the exception of the "detach" method is lifted from a ARX sample..  :-o

LE3

  • Guest
Re: ResbufList class
« Reply #3 on: March 17, 2010, 07:25:35 PM »
oops did not notice those were your initials...

was thinking on one of the projects I am working that will have the need to export several functions to autolisp. :)

Hi Dan,

First bird eyes looks good.

Once I get the chance to implement some autolisp functions again from one of my projects, will give a full try out - you should have written this class like seven years ago :).

Thanks! It's not only for lisp, it should be useful when working with XData, XRecords, Selection Set filters... or where you need to use a resbuf..


Is this yours? (I am asking because do not see your name on it).

of course its not mine, its yours, that's why I wrote it (note the "DRM" on the header)  :-D
...well with the exception of the "detach" method is lifted from a ARX sample..  :-o

pkohut

  • Guest
Re: ResbufList class
« Reply #4 on: March 17, 2010, 07:32:47 PM »
up for pier review  ^-^

Do you have some test code you can provide, that shows intended usage?

Code: [Select]
class CResbufList //derive from CObject or not??Gut says not.

At first I wasn't to crazy about, as it seemed a bit over baked and heavy for dealing with resbufs, but looking at it closer it has potential.  So these comments are purely after looking at it for about 15 minutes.

1) Eat the dog food you give your dog.  You provide the user of the class an iterator of begin() and end().  Use these in your loops as well, for instance the functions CResbufList::at, clone, and others.  You'll ensure they work as intended as well as provide users of the code ready made examples on how to use them.

2) Maybe add a size function to the class? Sorry just saw the length function, not a very good function name to conform with the std.

3) CResbufList::at and removeAt, kind of useless function taking size_t input parameters as there isn't a find function to know what index to delete.

4) CResbufList::at and removeAt, more useful if they took pointers or iterators at input parameters. (edit: added iterators)

5) Maybe add an insert function.

6) By inference of cloneNode your copy constructor and assignment operators are not exception safe. Make sure you check all API return codes and pointers, you've got a few de-referenced pointers that are not checked first.

Otherwise looks good Daniel.
« Last Edit: March 17, 2010, 07:36:30 PM by pkohut »

pkohut

  • Guest
Re: ResbufList class
« Reply #5 on: March 17, 2010, 08:10:07 PM »
Sorry just saw your sample usage at the top of the thread.

Function release leaves mpTail dangling.

Most of the functions returning resbuf pointers should return iterators instead.  I think that would solve a nagging issue with the class, and that's that the class does not encapsulate the resbuf pointer list enough. By returning iterators instead of raw pointers you are enforcing the notion that this is a container class and work to be done on resbuf lists should be done exclusively through the class (as much as possible).  To further enhance the utility of the container class, a CResbuf type class would be a useful addition, probably more useful than the container class itself.

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #6 on: March 17, 2010, 09:24:09 PM »

Great feedback! Thank you !!!!!
making changes...

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #7 on: March 17, 2010, 09:48:53 PM »
Item 6, how do I make the cctor and assignment operator  "exception safe"? , this is a nullable type.
There shouldn't be any dereferenced pointers  :-o

pkohut

  • Guest
Re: ResbufList class
« Reply #8 on: March 17, 2010, 10:14:36 PM »
You should check the return values of functions that return a newed buffer, acutNewRb acutNewString, etc.  Rule of pointers is never dereference a NULL pointer.  Look at cloneNode.

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #9 on: March 17, 2010, 10:17:16 PM »
You should check the return values of functions that return a newed buffer, acutNewRb acutNewString, etc.  Rule of pointers is never dereference a NULL pointer.  Look at cloneNode.

I see it now  :oops:

owenwengerd

  • Bull Frog
  • Posts: 439
Re: ResbufList class
« Reply #10 on: March 17, 2010, 11:52:58 PM »
I agree with Paul that you should keep the raw resbufs insulated from the client. I don't like your dependency on MFC. At the very least, I would make that dependency conditional so that the code can work correctly in a non-MFC project. Better would be to remove the dependency altogether.

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #11 on: March 18, 2010, 03:07:51 AM »
...I don't like your dependency on MFC. At the very least, I would make that dependency conditional so that the code can work correctly in a non-MFC project. Better would be to remove the dependency altogether.

ACK :-o  Do you mean toss the CString?

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #12 on: March 18, 2010, 09:05:07 AM »
Thanks again for all the feedback, I nixed the tostring and removeAt methods,
the functions that return resbufs are now private, the others return iterators.

gswang

  • Newt
  • Posts: 113
Re: ResbufList class
« Reply #13 on: October 25, 2010, 09:19:03 AM »
How to use ResbufList's remove method?
If i want to convert ResbufList_iter object into resbuf* , how to do it? :-o

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 6941
  • AKA Daniel
Re: ResbufList class
« Reply #14 on: October 25, 2010, 11:11:45 PM »
How to use ResbufList's remove method?
If i want to convert ResbufList_iter object into resbuf* , how to do it? :-o

You should be able to cast a ResbufList_iter to a resbuf