Delete Button Coding Best Practice

alanjfpaterson

I just have a few questions about best practice in relation to my new delete button code.

I am auto-generating some server code to handle the call from the client to calculate the number of references for a particular row (ie to actually use the methods that Christian is auto-generating.) I will be creating one file in each module that has delete functionality - at the moment this means MCommon, MFinance and MPartner but there will be more as we add more delete buttons. My file names are (for example) Partner.ReferenceCount-generated.cs.

So here are my questions....

I have created these files in the relevant "\Module\connect" folder. Thus I have created
csharp\ICT\Petra\Server\lib\MCommon\connect\Common.ReferenceCount-generated.cs
csharp\ICT\Petra\Server\lib\MFinance\connect\Finance.ReferenceCount-generated.cs
csharp\ICT\Petra\Server\lib\MPartner\connect\Partner.ReferenceCount-generated.cs

Are you happy with these new files being in these locations?? I did it because every module has a 'connect' sub-folder whereas not every one has a 'web' subfolder, for example, so there is no need for any smart mapping between module name and sub-folder name. In most cases the other files in the connect folder are the auto-generated instantiators.

Next question: I have chosen as the namespace for my new files
namespace Ict.Petra.Server.MCommon.ReferenceCount.WebConnectors
namespace Ict.Petra.Server.MFinance.ReferenceCount.WebConnectors
namespace Ict.Petra.Server.MPartner.ReferenceCount.WebConnectors

It has to be a namespace that ends with 'Connectors', but sometimes we use UIConnectors and sometimes WebConnectors. Is there a correct namespace name in this case?? I can easily change it to UIConnectors, if that is the correct philosophy.
As a related issue my class names are like 'TPartnerReferenceCountWebConnector'.

Next question....
For my two methods inside the class that return the reference usage I have assigned the RequireModulePermission attribute as NONE. Do you think that is ok? I justify that on the basis that the only caller for this method is a client screen for which authentication has already been given, but that could be a rather weak justification! I could easily add PTNRUSER for partner but if we need FINANCE permission you will need to explain to me whether that would be -1, -2 and -3 or what??

Final question....
I have coded my auto-generation code into GenerateGlue. It goes as the first task that GenerateGlue does - before it creates any interfaces (because of course I need interfaces creating from these new files). It could equally go in GenerateORM as the last thing that that application does. I chose not to do that because 1) Christian was working on GenerateORM and 2) after adding a new delete button to a screen (YAML file) it is necessary to run this code as well as the usual generateWinForm, so it is slightly less work to edit YAML and run generateGlue rather than generateORM AND generateGlue. But if you would prefer the new code in generateORM I can do that. I can see arguments both ways. (Note: having run generateGlue once after adding a delete button to a YAML file there is no need to do so every time the YAML is edited afterwards).

Thank you for your feedback. The good news is that the current code as-is completely works after a 'clean' and then a build from scratch. I just want to make sure that the new code fits in with your overall design vision.

alanjfpaterson

This conversation started as an e-mail. Timotheus replied to Alan as follows.....

I would very much prefer that you just write ORM code, and the existing
glue functionality picks up the webconnectors that you wrote.
Ok, rephrasing that after I have seen the code:
inc/template/src/ClientServerGlue/DataCountWebConnector.cs
that looks ok, but I would prefer you create that file from generateORM,
not from generateGlue.
The generate Glue creates the interfaces and the code that connects
client and server, based on existing webconnectors and UIconnectors, not
creating the server functionality itself.
I have rewritten the generateglue in my own branch, and therefore hope
you will not change too much in that.

For the UIConnectors, I think they are so specific, they should call
your web connectors manually, or you could call them from the template
form directly, as web connectors, even if the rest of the screen uses an
UIConnector.

For counting, the permission NONE is ok. For deleting, that is probably
another story.

By the way, please add copyright till 2013 to all your new files, and in
changed files as well.

alanjfpaterson

To Timo....

I would remove my private build from Jenkins for now. It is always going to fail because it needs Christian's code from his branch as well as the code from my branch. There are 7 files from him that I have manually copied onto my laptop but I am reluctant to commit them into my branch because I will end up with conflicts when I ultimately get his code from trunk. It might be a little while before Christian and I can merge all our changes together.

christiankatict

Hi Alan,

the Methods that you generate are really WebConnectors.

Thus they should not go into directories like "\Module\connect" folder (which are reserved for UIConnectors and the Instantiators), but into "\Module\web" directories (which is where WebConnectors ought to go). Because not every module has a 'web' subfolder you would need to create one if it isn't in existence yet.

The files should be called e.g. csharp\ICT\Petra\Server\lib\MCommon\web\ReferenceCount-generated.cs (no need to repeat the Module name in the file name as you had it).

I am happy with the names of the Namespaces and of the Classes.
As for the permissions I agree with Timotheus: For counting, the permission NONE is ok.

I also agree with Timotheus that the code should be generated by generateORM, not generateGlue, as this keeps the purposes of those generation tools apart. Adding your code generation to generateGlue really mingles things and it's not obvious to the user (=programmer) as to why this is.
I see the point that putting it into generateORM indeed will necessitate a three-step rather than a two-step process when adding a Delete button to an existing screen, but that can be automated using the OpenPetra Developer's assistants' 'Run Sequence' methods in case somebody does this to a lot of screens at once.

I have noticed that our wiki contained only a 'TODO' item where the purposes of the subdirectories under the 'Module directories' should have been described. That section now contains descriptions of each possible subdirectory:
sourceforge.net/apps/mediawiki/ ... tra.Server

Kind regards,

ChristianK

alanjfpaterson

Thank you Timo/Christian for your helpful feedback.

I will create the files in a 'web' folder instead of the 'connect' folder. They are already 'named' WebConnectors so that is ok.
I will remove the module name from the file name
I will move my code from Glue to ORM but will probably wait until Christian checks his branch in to trunk since he is working on ORM at the moment.

These were basically the issues that concerned me in the original post - I wanted to make sure that the code/files ended up in the right place. Thanks again...

I have found a 'bug' in TypedDataAccess which was the reason why my database returned 0 matching rows when I actually had 2. I will make a different post for that...

pokorra

Hello Alan,
I have disabled the build of your branch for now. You can easily enable it again (if you have a username and password for Jenkins, otherwise Christian should give that to you).

Timotheus

alanjfpaterson

Just a small update on 27 March....

I have now done as suggested above and moved my code to generateORM instead of generateGlue. The resulting auto-generated files no longer have the module name as a prefix to the file name. The files are now written to 'web' folders beneath the module (eg Server\lib\MPartner\web). The new 'web' folders have been added as required to the modules that did not previously have them.

I have now added more 'Delete' buttons to the screens - we are now up to a total of 77 screens with Delete!. While I have been going through the YAML files I have been adding an & to the label for the 'New' button as well as ensuring that the delete button label is &Delete. (I noticed that we had started doing this on newer screens but many of the Partner and Personnel screens were written a long time ago)

I have added a new target to the build - generateORMReferenceCounts. This is now also a dependency of generateORM. I have added generateORMReferenceCounts as another option for OP Developer's Assistant. The command line for the new functionality is like:

generateORM -do:referencecount -inputclient:${dir.root}/csharp/ICT/Petra/Client -outputserver:${dir.root}/csharp/ICT/Petra/Server -templatedir:${dir.incdir.template.src}

Obviously I could just use one path parameter (to csharp/ICT/Petra) if we are confident that we will always have the Client and Server paths in the same relation to one another.

When I do a clean and then a generateSolution everything compiles/works and I get the nice message box when I try and delete a referenced row - so that seems good.

I still have quite a lot of checking to do of the screens I have modified (all of the ..../Gui/Setup variety so far) and I haven't even started on any user controls.

I will not be checking any of this in to trunk until Christian has finished his current branch.

Let me know if you want me to change any of the target/build details......

christiankatict

Hi Alan,

this sounds all fine to me.

Kind regards,

ChristianK