Forum OpenACS Development: Use of inline functions in package creation scripts

I've found a lot of package creation scripts that create an inline function just to call another existing function, when they could just call that function right out of the script:
create function inline_0 ()
returns integer as'                                                                                    
begin                                                                                                  
    perform acs_object_type__create_type(                                                              
        ''forums_forum'',                                                                              
        ''Forums Forum'',                                                                              
        ''Forums Forums'',                                                                             
        ''acs_object'',                                                                                
        ''forums_forums'',                                                                             
        ''forum_id'',                                                                                  
        ''forums_forum'',                                                                              
        ''f'',                                                                                         
        null,                                                                                          
        ''forums_forum__name''                                                                         
    );                                                                                                 
                                                                                                       
    return null;                                                                                       
end;' language 'plpgsql';

select inline_0();
drop function inline_0();
As you don't do anything with the return value here, I wonder what's the reason for the use of that inline function. As a counter-example here's how Jeff does it in his Photo Album:
 select content_type__create_type (
   'pa_album',               -- content_type                                                           
   'content_revision',       -- supertype                                                              
   'Photo album',            -- pretty_name                                                            
   'Photo albums',           -- pretty_plural                                                          
   'pa_albums',              -- table_name                                                             
   'pa_album_id',            -- id_column                                                              
   null                      -- name_method                                                            
 );
I'm not saying these old scripts should be changed, since it doesn't harm anyone if they use inline functions. I'd just want to point out that you don't have to use inline functions in order to call another function unless you a) need to use the return value of that function later, or b) need to call that function in a loop for many entries.

If I haven't understood something totally wrong, that is 😊

Collapse
Posted by Randy Ferrer on

This methodology is unfortunately also being promulgated in the development tutorial for oacs here. There is generally little reason to create an inline function if it already exists and can be called directly as demonstrated by Jeff in his photo-album as you point out and by Lars in lars-blogger just to name two examples.

Collapse
Posted by Don Baccus on
Dan Wickstrom automated some of the porting of the datamodel from Oracle and PG.  I'm not 100% sure but this might be an artifact of automated porting ...

There's absolutely no need to do this, as has been pointed out, and our docs shouldn't suggest it.  You only need to do it if you need the result of one function call to pass as a parameter to another function.

Collapse
Posted by Jarkko Laine on
OK, I'll let Joel know about the docs...
Collapse
Posted by Dan Wickstrom on
Yes this is an artifact of automated porting, and it shouldn't be taken as a recommended practice for porting to postgresql.  Inline functions correspond to oracle inline code statements, and they end up being redundant in postgresql when a single function is being called.
Collapse
Posted by Joel Aufrecht on
Already in my todo list. I just wish somebody had told me the first time I asked. grumblegrumblegrumble. Maybe I should start a Development FAQ.
Collapse
Posted by Randy Ferrer on
A development FAQ would be nice, a development "best practices" document would be better... 😊
Collapse
Posted by Don Baccus on
Jun's written a "25 DOs and DON'Ts" which is a first cut in the direction of best practices ...

I'd like to help work on a best practices manual over the summer but until 5.0 is out most of our core folk won't have time, I know.

Collapse
Posted by Jun Yamog on
Hi Joel,

Would you like to take over the Do and Don't FAQ?  I will give them to you if you want.

Collapse
Posted by Joel Aufrecht on
Actually I'd like to just roll it into the tutorial.  But I guess that wouldn't help people who don't go through the tutorial.  But then how many people wouldn't read the tutorial but would read and absorb a Do and Don't?
Collapse
Posted by Jun Yamog on
Hi Joel,

Not sure.  But the Do's and Don't was geared towards developers with some experience with OACS.  Even some of the oldies would get some info out of it.  I made this up so we can finally have a definitive guide.  I think it was supposed to make it in 4.6.x but I think it was such an odd ball that it never made it.  So I guess maybe you can get it and see where it is best to put it into the docs.  So it finally goes in.

One of the reasons I stopped maintaining it actively because feedback slowed down.  It is likely since its only liked in this forums.  Much of the knowledge on this forums are buried after a while.  Since its coming up again, why not finally put it into the docs or something.

Collapse
Posted by Lars Pind on
I think we need both a tutorial and a reference manual. The do's and don's would be in the reference manual, but can be referred to from the tutorial as needed.

/Lars

Collapse
Posted by Dirk Gomez on
I think it is even more important to clean up the code because it is the prime reference to every programmer!

For example when I played around with the postgres version of the logger I just looked at other code and found the somewhat peculiar-looking stuff with inline functions. Not having a real clue of Postgres I just took up on that trick...

Collapse
Posted by Don Baccus on
If it is any consolation I've been removing the needless function wrapper when I've run across one in the course of doing other work.  I've not considered it worth the time to tackle it seriously.  The automated port technique enabled Dan to port an awful lot of code in a hurry - he did the kernel, content repository and CMS, that's a lot of datamodel to hack.
I agree that the code itself is the first point of reference.  So all we need is for all of the kernel code to precisely match the current conventions.  hahaha.  Okay, while we are waiting for the technology to clone Don (and hypnotise the clones into working on OpenACS), should we pick a single package to be the "perfect" package?  It should be one of the core packages; it should show a variety of different techniques; it shouldn't have any clever tricks; it should be fully code-reviewed by several people on a recurring basis; and the maintainer should commit to keeping it current with changing coding standards.