Forum OpenACS Development: Tipping of coding practices ?

I take it you missed the tcl style thread a month or two ago. :)

Hell, that wasn't even general style, that was just about whether we should follow the same proc parameter conventions as the rest of the tcl world...

I am rather sure that my proposals wouldn't be challenged.

Probably as long as you keep them to yourself they won't be. Another way to avoid that result is to keep the discussion so general no one knows for sure what the proposal is. Or just make changes without asking first.

I assume your coding practices are contained in the bullet list. As a comparison. The tcl coding practices are many pages in length and contain actual examples of good and bad style.

Mostly the bullets are way to general to be useful. But lets start by combining a few of your bullets. Are the formbuilder procs and the usage of the wrapper ad_form documented enough to require their usage?

Also, my long standing complaint about ad_form is that it encourages mixing html with the tcl pages, making it difficult to maintain code once it goes into a client project. I know folks are going to shout me down one more time on this, but when you put html attributes into the tcl page, that 'is' html code. Just because the '<' and '>' are not on the page doesn't mean you have removed html coding, thus presentation, from the tcl page. So one thing missing in 'use ad_form' is 'how to use ad_form', which I believe can be used in the way I am suggesting, but seldom is used that way.

Overall though, I don't understand the requirement to use ad_form and listbuilder. These are big complex tools that make the OpenACS toolkit more difficult to understand for new users. In contrast the db_* procedures simplify life. You don't need a rule to get folks to use them because you can understand them, most of the time, pretty quickly.

Code review: who can complain about that? Question: how do you determine that an api is 'unused'?

Your last idea of moving table/index apart from pl sounds good initially, but really in a lot of cases it isn't necessary. Indexes do have to be recreated if you make datamodel changes, maybe they should be separate from tables? Personally I haven't run into any problems in this area.

And what exactly is specific about 'document code'? I thought code was documentation. Documentation isn't needed until something doesn't work like you expect. Then you really need it. Chances are, the person who wrote the code understands the limitations and never ran into the problem, so it isn't documented anyway. Bottom line is that what the code writer thinks of as documentation might not be enough for the casual user. Better than just documenting code is to write it so it is easy to understand. We have a toolkit filled with procs that go on page after page. If this code was broken down into more understandable chunks it might be easier to figure on the occasion it doesn't work the way you think it should. You could also try using meaningful variable names. Also, not overloading a variable with several usages in a proc helps a lot in figuring out what a proc does.

I know I have a bunch of dumb ideas and views. I assume folks will question them. That is a good thing. Sorry to digress.

Dirk's point is a good one though, unless there are some people willing to do something besides just post on the forums about this it's unlikely to matter one way or the other. I will try to pull a draft together based on the various threads we have had in the past but I will be sorely disappointed if those of you who seem to care about this stuff are not willing to wade in and fix it.

The things can think of off the top of my head are datamodel create scripts, using namespaces, seperation of markup and code (and especially removing tcl from .adp files), package structure (which mostly needs to be updated, to be in line with tip 12), the ad_proc, ad_page_contract, ad_library javadoc tags, and declaring things public or private.

Also, I think we need a TIP for how we are going to remove deprecated functions (a la set_form_variables_string_trim_DoubleApos and util_PrettySexManWoman), since I would really like to clean out some of the really crufty stuff. I think remove anything two releases after it is -warn (i.e. remove the 4.6.3 -warn things in 5.1 and the 5.0 -warn things in 5.2) but that is probably something we need to talk about more and make a formal decision on.

RE specific draft guidelines. I certainly appreciate these guidelines. I have since "refactored" my own code to implement your proposed standard layout.

Thank you,

Randy

Instead of "document code" I would draft a guideline saying something like:

"Write clear, well commented code. Your comments in the code should explain WHY the code does something, wherever the why is not immediately obvious. If HOW the code does its work is especially tricky or confusing, then you should explain the how in a comment as well."

Jeff, yes, I take your and Dirk's point, but since I'm not going to volunteer (not anytime soon anyway) to write the coding standards doc and refactor lots of code, I might as well post to the forums. :)

Dirks point of cleaning up the code base is a very good one. I assume we might be able to do so during one of the OACS festivals with a lot of "worker bees" helping out. After all this is a good way for fresh developers to take a look at the code *and* learn the coding principles set forth.

My fear is that the principles will never be written down, unless I start to write this document. My hope is that Jade and Joel could devide this task between themselves. My goal is to not let this idea die and get documented coding principles out. Someday.

I think there's a thread a way back that describes putting that together.

i still think getting a bunch of people to agree on a required best solution is a sinkhole.  there needs to be some kind of motivation to define and follow the standard.  it needs to be loose enough to fluctuate based on the ideas of the community as a whole.

1.  define the group that will care about this stuff.
2.  get everyone to agree on a set of recommended standards for docs, code, and processes.  it doesn't have to be perfect, just something everyone can agree on at that time.  you can follow these, but you don't have to.
3.  if you choose not to follow the standards, you are an evil "unsupported outsider" unless you can build a case to the standards group for why your method should replace or be incorporated into the current standards.
4.  those that do follow the standards get community priority in support, placement, recomendations, and distribution.  this is heroic.
5.  the group requires periodic checking of existing code to make sure it fits current standards.  if not, you get bumped to evil-unsupported-outsider status.
6.  make the whole program very visible to the community.

I have given the Things to Remember to one of the guys that has doc privs.  I forgot, whom and when.  Anyway I would be glad to remaintain it as one of the pages here.  I also need to revise some of it.

Would it be worth be putting here?  If yes, can someone grant privs?  And where would we put it?