TheSwamp

Code Red => ARX Programming => Topic started by: It's Alive! on March 17, 2010, 11:43:18 AM

Title: ResbufList class
Post by: It's Alive! on March 17, 2010, 11:43:18 AM
updated
Title: Re: ResbufList class
Post by: LE3 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!
Title: Re: ResbufList class
Post by: It's Alive! 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
Title: Re: ResbufList class
Post by: LE3 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
Title: Re: ResbufList class
Post by: pkohut 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.
Title: Re: ResbufList class
Post by: pkohut 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.
Title: Re: ResbufList class
Post by: It's Alive! on March 17, 2010, 09:24:09 PM

Great feedback! Thank you !!!!!
making changes...
Title: Re: ResbufList class
Post by: It's Alive! 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
Title: Re: ResbufList class
Post by: pkohut 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.
Title: Re: ResbufList class
Post by: It's Alive! 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:
Title: Re: ResbufList class
Post by: owenwengerd 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.
Title: Re: ResbufList class
Post by: It's Alive! 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?
Title: Re: ResbufList class
Post by: It's Alive! 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.
Title: Re: ResbufList class
Post by: gswang 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
Title: Re: ResbufList class
Post by: It's Alive! 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
Title: Re: ResbufList class
Post by: gswang on October 31, 2010, 09:26:49 PM
命令: (resbuf_remove 1 2 3 4 5)
(1 2 3 4 5)     ;Not what i want. :-(

Quote
   static int ads_resbuf_remove(void)
   {
      ResbufList _list = acedGetArgs();

      ResbufList args = _list;
      ResbufList_iterator iter = args.begin();

      int i = 0;
      for(; iter != args.end(); ++iter)
      {
         if (i == 1)
            _list.remove(const_cast<resbuf*>(&(*iter)));

         i++;
      }
      
      acedRetList(_list);

      return (RSRSLT);
   }
Title: Re: ResbufList class
Post by: It's Alive! on November 02, 2010, 10:27:39 PM
Ah! I see the problem I will fix it tonight
Title: Re: ResbufList class
Post by: gswang on November 02, 2010, 10:46:01 PM
Thank you Daniel, recently busy, right? :-)
Title: Re: ResbufList class
Post by: It's Alive! on November 03, 2010, 09:12:17 PM
Try it now  :-)

Code: [Select]
// (rbtest 1 2 3 4 5 6 7 8 9 0)
  static int ads_rbtest(void)
   {
      ResbufList args = acedGetArgs();

      //added this
      args.removeAt(1);

      ResbufList_iterator iter = args.begin();
      int i = 0;
      for(; iter != args.end(); ++iter)
      {
        if (i == 1)
             args.remove(iter);
         i++;
      }
      acedRetList (args);
      return (RSRSLT);
   }
Title: Re: ResbufList class
Post by: It's Alive! on November 03, 2010, 09:14:57 PM
BTY, I changed a couple of function names  :whistle:
Title: Re: ResbufList class
Post by: Kerry on November 04, 2010, 12:17:07 AM
BTY, I changed a couple of function names  :whistle:

 .. and here's the documentation :) (http://www.theswamp.org/index.php?topic=32574.msg408141#msg408141)
Title: Re: ResbufList class
Post by: It's Alive! on November 04, 2010, 01:28:03 AM
I know, I know.... but I did comment the code (a little)  :|
Title: Re: ResbufList class
Post by: It's Alive! on November 04, 2010, 01:37:58 AM
I changed push_back(resbuf*)  to only accept a single node (for performance) and added  append_front(resbuf*) and append_back(resbuf*)  for chains.
Title: Re: ResbufList class
Post by: Kerry on November 04, 2010, 04:36:07 AM
I know, I know.... but I did comment the code (a little)  :|

It's cool Daniel ... I'm just stirring  :-P
Title: Re: ResbufList class
Post by: gswang on November 10, 2010, 06:32:18 AM
 :?The push_back function could not work correctly!
ResbufList returnBuffer;
for(size_t idx =0; idx<points.length(); idx++)
{
resbuf *pTmp = acutNewRb(RT3DPOINT);
pTmp->resval.rpoint[0] = points[idx].x;
pTmp->resval.rpoint[1] = points[idx].y;
pTmp->resval.rpoint[2] = points[idx].z;
returnBuffer.push_back(pTmp);
}
Title: Re: ResbufList class
Post by: It's Alive! on November 10, 2010, 07:11:43 AM
fixed, try it now
Title: Re: ResbufList class
Post by: frtfff on December 24, 2010, 10:15:21 PM
 :-(
Title: Re: ResbufList class
Post by: It's Alive! on December 24, 2010, 10:24:56 PM
why the long face?
Title: Re: ResbufList class
Post by: pkohut on March 24, 2011, 09:46:03 AM
A year after the comments and I finally get to take at look at the changes you made. Looks good Daniel.
Code: [Select]
inline ResbufList_iterator ResbufList::begin()
{
  return (mpHead == NULL ? NULL : mpHead);
}
Compiler will probably optimize to:
   return mpHead;
but if not then it's a wasted if statement.


The loops structures in ResbufList::at and ResbufList::removeAt can become -
Code: [Select]
ResbufList_iterator ResbufList::at( const size_t idx )

  for(size_t nItems = 0, resbuf *pRbTemp = mpHead ; pRbTemp != NULL ; pRbTemp = pRbTemp->rbnext, ++nItems)
  {
    if(idx == nItems)
      return pRbTemp;
  }
  return NULL;
}
Title: Re: ResbufList class
Post by: frtfff on March 24, 2011, 11:32:52 PM
Good code,but I don't know how to use.
Title: Re: ResbufList class
Post by: pkohut on March 25, 2011, 12:48:09 AM
Good code,but I don't know how to use.

See this post for ResbufList in use.
http://www.theswamp.org/index.php?topic=28286.0
Title: Re: ResbufList class
Post by: It's Alive! on March 25, 2011, 06:57:54 PM
Thanks Paul!

On your second entry, will that even compile? I always thought those args in the for loop had to be of the same type.  :mrgreen:

Dan
Title: Re: ResbufList class
Post by: pkohut on March 25, 2011, 07:06:40 PM
You're right. The declaration of nItems should be outside, however the assignment will still work inside.

Code: [Select]
size_t nItems = 0;
for(resbuf *pRbTemp = mpHead ; pRbTemp != NULL ; pRbTemp = pRbTemp->rbnext, ++nItems)
{
    if(idx == nItems)
        return pRbTemp;
}

... do lots of work

for(nItems =0, resbuf *pRbTemp = mpHead ; pRbTemp != NULL ; pRbTemp = pRbTemp->rbnext, ++nItems)
{
    if(idx == nItems)
        return pRbTemp;
}