cslaforum icon indicating copy to clipboard operation
cslaforum copied to clipboard

Consider requiring data portal operation methods to be public

Open rockfordlhotka opened this issue 5 years ago • 5 comments

Looking forward to CSLA 6 (which will be this fall, with .NET 5 support), @JasonBock and I were discussing a fairly substantial change and I'd like to solicit feedback/input.

Consider requiring data portal operation methods to be public

rockfordlhotka avatar Apr 13 '20 22:04 rockfordlhotka

First a disclaimer; I just started looking at Android, iOS apps using Csla : ) If making data portal methods public would help solve #929 , then I am all for it! Kind regards

Chicagoan2016 avatar Apr 13 '20 22:04 Chicagoan2016

If we have an analyzer to assist in migrating, as described in https://github.com/MarimerLLC/csla/issues/1562 There will be no issue on my side.

But why not making them 'protected internal' instead of 'public'? We already made those protected / accessible only to our unit tests DLLs.

Gilles

-------- Message d'origine -------- De : Rockford Lhotka [email protected] Date : 14/04/2020 00:03 (GMT+01:00) À : MarimerLLC/cslaforum [email protected] Cc : Subscribed [email protected] Objet : [MarimerLLC/cslaforum] Consider requiring data portal operation methods to be public (#930)

Looking forward to CSLA 6 (which will be this fall, with .NET 5 support), @JasonBockhttps://github.com/JasonBock and I were discussing a fairly substantial change and I'd like to solicit feedback/input.

Consider requiring data portal operation methods to be publichttps://github.com/MarimerLLC/csla/issues/1562

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/MarimerLLC/cslaforum/issues/930, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA3T6GL73MSEOMA3HEM3WITRMODZPANCNFSM4MHIOZPA.

GillesBer avatar Apr 14 '20 05:04 GillesBer

I don't think this is a good idea, personally. I like Csla because it helps encourage good OO design, but making something public just to get rid of a warning which is easily disabled seems like a bad tradeoff. Marking something private is the built-in way of saying "don't call this externally." Plus the method would start showing up in intellisense correct, even though the analyzer would flag calling it?

I thought the reflection results were cached as well, so isn't the already small reflection hit only happening once?

We get the same unused code warnings on all the private setters for read-only business objects, because the data portal methods are compiled out of the version of the business assembly the clients use, I don't see how this is much different.

ajj7060 avatar May 06 '20 23:05 ajj7060

I agree also, not a good idea. We try to hide things from the consumers of our Library as much as possible (UI developers, people that use our library as an API, etc.). This simplifies their learning curve and eliminates a lot of errors. The less that's there that doesn't work for them anyways, the better, IMO.

brinawebb avatar May 07 '20 03:05 brinawebb

First a disclaimer; I just started looking at Android, iOS apps using Csla : ) If making data portal methods public would help solve #929 , then I am all for it! Kind regards

We encourage using factory methods/criteria instead of DataPortal methods. To us, the criteria is a contract between the factory method and the dataportal method. This works very well. I realize that they may need to be exposed, but our coding standard dictates that DataPortal.Crud calls are only done within the class that owns it.

brinawebb avatar May 07 '20 03:05 brinawebb