Forum OpenACS Development: Re: content_folder.rename

Collapse
4: Re: content_folder.rename (response to 1)
Posted by Samer Abukhait on
reported as bug #1193
Collapse
Posted by Dirk Gomez on

Samer, thanks for pointing this out. An early aD style guide recommended the use of <proc_name>.variable_name.

I think we should just drop this coding style recommendation and move towards to variables with prefixes e g. v_ or m_. I agree that the aD style guide looks nicer and cleaner, but it is error-prone and it is next to impossible to track down these errors except with constant code review. And why not just avoid bugs that are hard to be tracked down - if it is so easily done?

Collapse
7: Re: content_folder.rename (response to 4)
Posted by Dirk Gomez on
The problem is that PL/SQL needs to tell apart variables from column names. If the variable name happens to also be a column name, you must prefix it with the proc name, otherwise you will have a clause that is short circuited.
Collapse
9: Re: content_folder.rename (response to 4)
Posted by Dirk Gomez on

Here's a good example where I think that this naming convention under Oracle is dangerous:


        procedure del (
                calendar_id             in calendars.calendar_id%TYPE
        )
        is
  
        begin
                  -- First erase all the item relate to this calendar.
                delete from     calendars 
                where           calendar_id = calendar.del.calendar_id;
 
                  -- Delete all privileges associate with this calendar
                delete from     acs_permissions 
                where           object_id = calendar.del.calendar_id;

                  -- Delete all privilges of the cal_items that's associated 
                  -- with this calendar
                delete from     acs_permissions
                where           object_id in (
                                        select  cal_item_id
                                        from    cal_items
                                        where   on_which_calendar = calendar.del.calendar_id                                                                                                                                                         
                                );
                        
 
                acs_object.del(calendar_id);
        end del;

It admittedly is more stylish, but it is error-prone (without throwing errors) and is potentially dangerous because it may delete loads of items.

Let me dig up a coding style proposal from Feuerstein's books once I am back from the Hamburg bug bash.