Forum OpenACS Q&A: Top 20 DOs and DONTs while developing OpenACS

Hi,

I would like to make a Top 20 list of what you should do or dont while developing in OpenACS. Its goals are:

  • Help new and old developers about standards in a short doc.
  • Help developers read the real docs since the list will link to the real docs if possible.
  • Hopefully start a new chapter or new doc if one items is missing from the real docs we have.
  • Hopefully I make a good contribution with least effort.
  • The list will set a good guidline for future developments. And maybe the older code will change in time.

It all started with this thread. I am sure there are other older threads that should have made it into the doc.

To make things simple we will first just have the first 5, after it has been establish we will get the next 5 and so on.

Here is the first five that will make it into the list if nobody objects: (I will add more detail on the real doc)

  • Use Namespace (ex. foo::get_title, etc.) blah blah.
  • Use return_url as the var name if it will be used to return url... blah blah.
  • Avoid putting in tcl logic on adp pages if possible. Use foo.tcl and foo.adp. The same way is true for tcl... avoid putting in html on tcl.
  • Use package_key__foo or package_key.foo for db api. Use package_key::foo for tcl api.
  • Use ad_form... blah blah.

If all 5 items are valid and correct I will put them in. Also if anyone can put a good 1 - 2 liner write up on any items above please post them up. A link to the documentation is also desired. I am more that happy to use them rather than come up with my own.

Collapse
Posted by Vinod Kurup on
This would be very helpful, Jun. It belongs in this section (https://openacs.org/doc/openacs-4/eng-standards.html) of the docs.

A couple other possibilities:

  • Avoid the PG view/sequence trick
  • Delete the queries from your tcl files
  • When calling PL/PGSQL functions with more than 3 or 4 params, put a comment next to each param with the param's name
  • When creating PL/PGSQL functions, use p_ as the prefix for supplied parameters and v_ for local vars.
  • When declaring local PL/PGSQL vars, don't declare the type explicitly, instead use the table_name.column_name%TYPE notation.
        v_revision_id     cr_revisions.revision_id%TYPE;
    
    Collapse
    Posted by Jun Yamog on
    Thanks very much Vinod.  I will use your list as the next 5.

    Also it seems that Namespace is already validated by Yon on the namespace thread.  So 4 to go... and next batch of 5 will be next.

    a couple more:

    1) use named parameters whenever possible

    2) use package_instantiate_object and relation_add, although i think these functions have to be moved since, for reasons unbeknownst to me, they are in acs-subsite/tcl/package-procs.tcl and they should probably be in acs-tcl/tcl

    Collapse
    Posted by Jun Yamog on
    Thanks Yon.

    I will put that for the 3rd batch.  I welcome new items please don't get me wrong.  But would like to validate first the first batch.  Don't want to put in the doc that is wrong.

    Can someone validate the first 5?  After 2 days I might put those into writing and move on to Vinod's list.

    Jim I don't understand #4 of your list: "Use package_key__foo or package_key.foo for db api. Use package_key::foo for tcl api" The second sentence seems to be a subpoint of #1 - use Namespace - telling folks how to name their Tcl namespace. Is the first part a similar concept but specifying a query dispatcher pseudo-namespace?
    I think the document would be especially helpful (to me anyway) if the items (conventions and standards) are organized by context: tcl (programming, as used in adp), SQL (pgSQL, SQLplus) etc.
    Collapse
    Posted by Jun Yamog on
    Hi Cynthia,

    #4 is related to #1 although the essense of #4 is using the package key since I have encountered so many times people not using the package key and use some shortcut for those.  I think future app should make use of package key rather another shortcut.

    Torben,

    Ok I will categorize it that way after the 20 items are in.  This will be the initial arrangement and lets see if things will work.

    Collapse
    Posted by Stephen . on
    These are bugs; report them and/or fix them.  Some of it has already been silently ignored in the document Vinod pointed to, what difference will a new list make?

    The golden rule is consistency, and breaking it is absolutely unacceptable.

    Collapse
    Posted by Jun Yamog on
    Torben,

    Thanks for the link.

    Stephen,

    You are right that these are bugs and inconsistencies.  I am not saying that this doc will cure it.  I just wanted to try something that MIGHT be helpful.

    Collapse
    Posted by Jun Yamog on
    Hi,

    Here is the initial doc...

    20 Things to Remeber when Developing on OpenACS

    Its still is fuzzy but I hope its off to a good start. I would need help on this. Especially from Vinod and Yon.

    Please post your commnents, corrections or better yet... Post a link to an edited version! Thanks.

    Collapse
    Posted by Lars Pind on
    Great start, thanks, Jun.

    One comment: Do not use "==" in comparing strings:

    use [string equal ..] instead of [string match ...]

    /Lars

    Collapse
    Posted by Jun Yamog on
    Thanks Lars.

    Its so obvious that I am an offender of that guideline... :)

    Hi Lars,

    I was aware of the == problem, but I've been using [string match ...] all over the place! Why do you suggest "equal" instead of "match"?

    Collapse
    Posted by Lars Pind on
    The reason we *weren't* using equal originally was that it didn't exist. string equal was introduced with Tcl 8.

    string match does glob matching, with * and ? and [...] and that stuff, whereas string equal does an exact match. So (a) it's more precise, if what you want is straight-up-no-wildcards string comparison, and (b) it's presumably faster (I haven't looked at how it's implemented, but unless it's implemented using string match, you'd think that you'd save a few comparisons and special-cases for * and ? and [...] here and there).

    /Lars

    Collapse
    Posted by Stephen . on
    The section about Tcl in ADP's and HTML in Tcl code; how about some examples or alternatives here?  How should people achieve this?  Probably need something about when and why to register ADP tags, for example instead of calling a proc in an ADP; and when to use registered tags or included templates, instead of embedding HTML in Tcl.

    The thing about return_url comes under the more general heading of naming.  For instance should it be mentioned that boolean variables have a _p affix?

    When deleting queries from Tcl files perhaps we could replace the query with the literal string *SQL* rather than "" or {}.  If you make a mistake and the correct query can not be found from the .xql file, the blank error message can be a little confusing, but with *SQL* it's more obvious what has happened.

    Why must plpgsql function parameters be prefixed with p_?  I don't remember seeing this used already in the toolkit, and we don't prefix Tcl parameter names with p_.  Variable names are prefixed with v_ because you can't assign a new value to a function parameter (right?) and so must use a new but simillarly named variable.

    I know this is just a draft, but ultimately there probably needs to be a complete document describing how to create upgrade scripts, with a simple link to it explaining that it must be followed.  I think Jon Griffin's your man here...

    Collapse
    Posted by Jun Yamog on
    Hi Stephen,

    Thanks for the great feedback.

    1. Tcl on Adp.  I have placed a short example of that on my initial draft.  Although it would be great if you can point me to where I can make my own ADP tags.  I am not aware of anything in the doc that talks about this.  I have just place my comment about this on the openacs.org docs section.

    2. _p affix is something I forgot.... I will add it there.  Maybe I should move return_url and _p to the "others" section.  And maybe rename others as general.

    3. Putting "SQL" seems to be more logical.  I will add that too.  I someone objects please do... otherwise no objection means acceptance.

    4. p_ in plSQL is much better.  I have used that several times.  I think it makes it less confusing.  For example you pass in item_id.  Way down the code I am sure you will use item_id local var.  So making item_id to p_item_id and v_item_id make it easier to know which is which.  As far as tcl it don't seem to know why don't get a problem on it.... I don't know exactly why.  Maybe its some odd norm.

    Thanks for the feedback its great appreciated.

    Collapse
    Posted by Jade Rubick on
    How about adding in the $ID tags for CVS? That might be a good
    addition as well. I'm not sure if this is in the documentation
    already.
    Collapse
    Posted by Jun Yamog on
    This is just to note John's suggestion to include version numbers on the list.

    at this thread:

    https://openacs.org/bboard/q-and-a-fetch-msg.tcl?msg_id=0005WY&topic_id=12&topic=OpenACS%204%2e0%20Design

    Sorry about the extra post.  Just would like put into the this thread  anything that is related to it.

    Collapse
    Posted by Jun Yamog on
    I will update the list.  I am a bit busy now.  But I would like to add another for your comments.

    When using upvar name your var as packagekey_varname. Example:

    upvar cms_item_id cms_item_id

    That way its unlikely you will step on someone's var.  Another related to upvar.... if can use another method, avoid using upvar.

    Collapse
    Posted by Jun Yamog on
    I have updated the list to include the added suggestions. And finished up some items too.

    Grab it there

    Please help on the comments that is in dark red.