-
-
Notifications
You must be signed in to change notification settings - Fork 6
Blazor ViewModel additional functionality (3 questions) #938
Comments
j055, I ran into the same problem with the Child PropertyInfo's. Here is an example I used in Blazor to get a Child's PropertyInfo
ChildToEdit is the specific child instance in the Child Collection. I used this in an editable grid. I guess the ViewModel could provide that same option, but ViewModel is also caching the PropertyInfo objects in a dictionary by PropertyName. Doing that for Child Collections might be hard to do in the Root Model. Ben |
|
|
|
Regarding number 4, I am creating a new |
Regarding number 1 - await vm.RefreshAsync(() => DataPortal.FetchAsync<PersonEdit>(123)); This eliminates all ambiguity, making the code explicit and more readable. |
I've addressed items 2 and 3 with a new var info = vm.GetPropertyInfo(() => vm.Model.Child.Name); Basically, you can provide a |
If this were required then the factory methods that we've created would be bypassed. What if your object depended on getting data from the database and called a DataPortal.Create to load some other data, before the DataPortal.Fetch? FWIW - we rarely (if at all) call a DataPortal method from outside the class as it bypasses our factory methods... |
I probably won't commit the PR until the end of the weekend, so if you'd like to review the code please check out the PR> |
@brinawebb The new |
Thanks @rockfordlhotka . I misunderstood, thought you were implying that 'required' meant that's the only way. |
Yes, confirmed - either works. Page code is like this: protected override async Task OnParametersSetAsync()
{
//await vm.RefreshAsync(
// () => Csla.DataPortal.FetchAsync<PersonEdit>(123, "Rocky"));
await vm.RefreshAsync(
() => PersonEdit.FetchPersonEditAsync(123, "Rocky"));
} |
@brinawebb well, I am removing the You wouldn't have used the old |
fwiw, I'm generally not using or recommending static factory methods anymore, because they are a blocker for unit testing. I don't plan to stop supporting them, I just don't plan to incorporate them in the new editions of the books. |
Gotcha, thanks for noting that. We were just creating a set of coding guidelines on a new product that preferred the factory methods so it's good timing for us to not do that at all. Odd, we have a lot of unit tests and don't have any troubles. Probably a separate topic, but what blocks them? Are they unit tests on top of the Csla project? |
Yeah, if you try to unit test the domain classes it is hard to create them using static factory methods. Well "hard" is relative, because you can create a mock client-side data portal. But if you want to mock the domain types then you might not want to be creating them through a factory, because that gets overly complex compared to using Rocks or Moq or whatever. @JasonBock is kind of my go-to expert in this area. |
Gotcha. Yes, from the business objects on up we don't have any problems with unit tests. We think of the library as having any UI on top of it, including commandline utilities as well as encouraging our customers to use it as an API. Sometimes we'll put Json web services on top. To us, the unit tests are just another UI. However... I'm a bit perplexed that you're not recommending factory method usage anymore as they play a HUGE role in the library. I realize that it's a factory pattern, and isn't really a Csla thing, but it is a very important logical layer (or teir? not sure if I have the correct terminology). The factory methods simplify development for the tier above as well as the DataPortal methods in the tier below. I think of them as a layer between the business object and the CRUD operations. It is a very simple and thin layer, and my discussion here might make it sound complex, but we do rely on a lot of different things here (oftentimes not all at the same time). This tier is where our criteria live (if we need them) and we don't expose them to the tier above. Only the DataPortals and the factory methods use them. This allows us to not only centralize our input/parameter validation, but we can fetch or create other things that are needed, use other complex criteria, setup business rules, etc. Using this tier in this fashion also protects our DataPortal methods from having to do all sorts of invalid parameter checking, etc., which you are forced to do if you call them directly. Parameter validation in the DataPortal methods puts logic in the wrong place IMO as it sometimes forces you to use an exceptions if you want to communicate back (very naughty). The DataPortals in our library are clean and just do what they're supposed to do (talk to a database). For us, if a parameter is wrong, we go to the factory/criteria method to fix it. Also, just from a developer productivity standpoint, centralizing the create/fetch of an object makes it simpler and there are less bugs. I realize that in the end the factory methods are just a software pattern, not really a Csla thing. This thread was good as it forced me to think about how we create our library and validated that we're on the right path. I have decided to keep on our current path and use the factory methods as the only way to create instances of our objects. Unfortunately there's no way to enforce it, but there are a lot of things like that in life :-) Brian |
Hi everyone, I am late on this thread but I would love to see the samples or project tracker without factory methods, I have always used the factory methods approach. |
@brinawebb there's a third alternative to consider as well. Use static factory methods [Serializable]
public class PersonEdit : BusinessBase<PersonEdit>
{
public static async Task<PersonEdit> GetPersonEditAsync()
{
return await DataPortal.FetchAsync<PersonEdit>();
}
} usage: var obj = await PersonEdit.GetPersonEditAsync(); Use data portal directlyNo setup, just usage, which is nice - keeps the code clean at the cost of abstraction. var obj = await DataPortal.FetchAsync<PersonEdit>(); Use factory type public interface IPersonEditFactory
{
Task<PersonEdit> FetchAsync();da
}
public class PersonEditFactory : IPersonEditFactory
{
public static async Task<PersonEdit> FetchAsync()
{
return await DataPortal.FetchAsync<PersonEdit>();
}
} usage: IPersonEditFactory factory = new PersonEditFactory();
var obj = await factory.FetchAsync(); Now let's consider using dependency injection of these types. Use static factory methodsNot possible, because there's nothing to inject. All you can do is use the factory method. Makes it harder to unit test things like a viewmodel. public class PageOrSomething
{
public void OnInitialize()
{
this.DataSource = await PersonEdit.GetPersonEditAsync();
}
} Use data portal directly public class PageOrSomething
{
private readonly IDataPortal<PersonEdit> portal;
public PageOrSomething(IDataPortal<PersonEdit> dp)
{
portal = dp;
}
public void OnInitialize()
{
this.DataSource = await portal.FetchAsync<PersonEdit>();
}
} Config in startup: services.AddTransient<IDataPortal<>, DataPortal<>>(); Use factory type public class PageOrSomething
{
private readonly IPersonEditFactory factory;
public PageOrSomething(IPersonEditFactory f)
{
factory = f;
}
public void OnInitialize()
{
this.DataSource = await f.FetchAsync();
}
} Config in startup: services.AddTransient<IPersonEditFactory, PersonEditFactory>(); |
Like all the proposed changes above. Saw this and thought there was a better way to create a key from an expression (a bit of OCD due to lockdown).
use instead
This is about 20% faster. |
Interesting. I wonder if that's due to calling var keyName = property.ToString();
keyName = keyName.Substring(keyName.IndexOf(").") + 2); |
Mine is still 11% better. You don't actually need to do the |
I don't remember what is before the ")." in the string, so you might be right that the I think it is interesting though, that their If I may though, my criticism of your code is that it seems like it would be better implemented with a |
You might be right so for completeness here's a
Your |
Oh, that's funny! I took your stringbuilder code and though to myself it would be easier (maybe faster?) to use So in the actual codebase I'm using your original |
Question
RefreshAsync
seems a bit opinionated. For example, if I want to copy a BO by passing in an IDRefreshAsync
will try to do a 'fetch' instead of a 'create'. Wouldn't it be better to have CreateAsync and FetchAsync methods instead ofRefreshAsync
?GetPropertyInfo
can only handle properties of the root object. Should there be an overload method, e.g.GetPropertyInfo(target, propertyName)
or another way to get property info in child objects?Related to above - what's the difference between:
@if (vm.Model.CanReadProperty(nameof(vm.Model.Name)))
and
@if (vm.GetPropertyInfo(nameof(vm.Model.Name)).CanRead)
and why is one better than the other?
Version and Platform
CSLA version: 5.1
OS: Windows
Platform: Razor components (Blazor)
The text was updated successfully, but these errors were encountered: