Virtue SKILL and Python Library

Hi,

This is a nice idea, I wish I could do the same but unfortunately the code I develop is not my propriety...

I looked at your source code and I have seen some things I believe you could improve :

Firstly, in your "packages", you recreate a DPL at the end of each let, you could get rid of rewriting the name of all the functions you define by using environments instead

;; Equivalent of what you do
package = (let nil
  (defun a nil nil)
  (defun b nil nil)
  `(nil a ,a b ,b)
   )

;; Could be
package = (let nil
  (defun a nil nil)
  (defun b nil nil)
  (theEnvironment)
  )

package->??
(((a funobj@0x3dafd368) 
    (b funobj@0x3dafd390)
    )

This function made me tick:

procedure(assocKeys(assocList "l") let(((out nil))
    "Returns all the keys in the specified association list"
      foreach(item assocList
          out = tconc(out car(item)))
      out
  ))

When you put a let as the second procedure argument instead of the docstrings you write rigorously, you bypass the native (and LISP conventional) docstring mechanism which is unfortunate if later you want to automate the documentation generation in SKILL for instance

Also you do (let ((out nil)) ... ) which is equivalent to (let (out) ... ) why be so meticulous when you could be lazy

You use tconc which is a nice idea because it is an underused and powerful function, except it makes no sense in your case.
You are already looping over your list: foreach mapcar or mapcar only will do the job for you.
Also you don't return the car of your tconc structure so you are not exactly returning what your docstring is describing

;; The following solutions are completely equivalent:
procedure(assocKeys(assocList "l")
    "Returns all the keys in the specified association list"
    foreach(mapcar item assocList
          (car item)
  ))

procedure(assocKeys(assocList "l")
    "Returns all the keys in the specified association list"
    mapcar( 'car assocList)
  )

The following function for instance could make a much better use of tconc :

procedure(uniqueListOrdered(in "l") let((item out noAdd)
    "Returns a list of items of the unique elements from the input list 
     while keeping the order between the first unique elements"
      out = '()
      foreach(item in
          noAdd = member(item out)
          unless(noAdd
              out = append(out list(item))
          ) 
      )
  out
  ))

;; Way faster using tconc as append rebrowse your list every time you loop in foreach
procedure(uniqueListOrdered(in "l")
    "Returns a list of items of the unique elements from the input list 
     while keeping the order between the first unique elements"
    let(((out tconc(nil nil)))
      foreach(item in
          unless(member(item cdar(out))
              tconc(out item)
          )
          )
  ;; Return list instead of tconc structure
  cdar(out)
  ))

I haven't had time to investigate further but this thanks for your share

Hope this helps

Cheers

Aurel

Het Aurel:
Thanks for the feedback!  

Packages

Thanks for the good suggestion!  This will be particularly helpful for the test framework where I would definitely prefer not to have to add each function to a DPL.  I need to move on to some other issues, but I've recorded this as a GitHub issue to address later.

It could also be helpful for the packages as you suggested.  We could handle the concern that the entire environment would then be available by using a naming convention such as _FunctionName() for private functions, similar to Python.  I do like how the current DPL method outlines what functions are available.

assocKeys

I've fixed the assocKeys function and added a test for it.  It definitely needed to be revised. 

Thanks,

Curtis

Het Aurel:
Thanks for the feedback!  

Packages

Thanks for the good suggestion!  This will be particularly helpful for the test framework where I would definitely prefer not to have to add each function to a DPL.  I need to move on to some other issues, but I've recorded this as a GitHub issue to address later.

It could also be helpful for the packages as you suggested.  We could handle the concern that the entire environment would then be available by using a naming convention such as _FunctionName() for private functions, similar to Python.  I do like how the current DPL method outlines what functions are available.

assocKeys

I've fixed the assocKeys function and added a test for it.  It definitely needed to be revised. 

Thanks,

Curtis

AurelBuche Do you have a GitHub account?  Would you be interested in trying out the library?

I do but I cannot use my personal account at work and even if I enjoy coding in SKILL I don't usually do it on my personal computer which has no access to Virtuoso