|
Thats where I thought you were going.
Then I thought, right now, my entities are simply made up of auto-implemented properties. This means I have to implement each property. That's a hell of alot of code to add to each entity, especially considering my entites are going to be used in an MVC3 app as well as my WPF app. That would mean putting WPF-specific code in classes use for non-WPF platforms.
This doesn't feel right.
If it's not broken, fix it until it is
|
|
|
|
|
How have you autogenned those properties? If it's Entity Framework for instance, it can auto gen the prop change code for you.
|
|
|
|
|
No, simply
public string Caption {get; set; }
How their generated is irellevant here. The entities are in a seperate Entities project used by 2 diferent UI projects - WPF and Web.
So now we're talking about going from
public class AddressEntity : _EntityBase
{
public string Caption { get; set; }
public string Street1 { get; set; }
public string Street2 { get; set; }
public string City { get; set; }
public string State { get; set; }
public string ZipCode { get; set; }
}
to
private string _Caption = string.Empty;
public string Caption
{
get { return _Caption; }
set
{
if (_Caption != value)
{
_Caption = value;
RaisePropertyChanged("Caption");
}
}
}
.
.
.
.
}
That's a tremendous amount of code in many entities just to support WPF.
One possibility is in the WPF UI simple reload the list from data.
If it's not broken, fix it until it is
|
|
|
|
|
Kevin Marois wrote: How their generated is irellevant here
Even I beg to differ here, a good code generator is priceless. I believe EF 4 does this but I wrote my own many years ago and would not work without it!
You basic issue still exists and is one I am contemplating, UI code in your entity (model in my case) classes that is used by a non UI consumer. This is precisely what we do, the WCF and the Silverlight UI reference the same model which has fully implemented properties.
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
My point was, whether generated by a tool, or typed in my me, is irellevant.
If it's not broken, fix it until it is
|
|
|
|
|
Speaking of EF - I spent some time over the weekend looking at EF, as I hadn't used it before. Seems simple, but the Entities seem heavy compared to the POCO's I'm currently using.
Also, assuming the DAL is going to be hit via a WCF service, would you use EF entities to transports the data? This really seeme smore suited to POCO's.
If it's not broken, fix it until it is
|
|
|
|
|
I haven't used EF, or even looked at it. I went on the feedback over the last few years that it was buggy and could be difficult to use. With the release of v4 apparently they have improved to a usable product, however I am reluctant to throw away my current framework and I have no intention of re factoring 13 projects currently in production or UAT!
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
Kevin Marois wrote: That's a tremendous amount of code in many entities just to support WPF.
One possibility is in the WPF UI simple reload the list from data.
As Pete mentioned, your AddressEntity needs to implement INotifyPropertyChanged. Well, actually, I hope you have all that encapsulated in a base class and you just derive everything from ObservableObject or whatever .
In regards to your idea of reloading the list, that'll only work on the surface level, but it'll break a LOT of things. Lets say you have a collection tied to a ListBox or whatever and the user has Item5 selected and clicks the edit button and changes the name to Item5a. The way you have it now, the ListBox will continue to display Item5 because INotifyPropertyChanged was not implemented. So you'd try to refresh the entire list, now you'll run into an issue where Item5a has lost its selection, the h & v scroll positions will be lost as well, etc. Not a good UI when that happens .
Anyways... if you aren't using EF, how many properties are we talking about? 5, 10, 50, 100?
I think if it was 5 to 10, I would just hand job it with c&p... but if it started to get closer to 50 or 100 or more, I would probably just write a quick & dirty C# app that loaded up your .cs file and simply parsed each line. The return type would be the 2nd value and the property name would be the 3rd value. Your app can then auto-gen all the WPF code. Probably a 10 to 20 minute job tops to whip out a C# app that does that.
Nothing wrong with a non WPF app using the code since the PropertyChanged event will be null, your handler will just return.
|
|
|
|
|
I know all this, and I considered adding in the RaisePropertyChanged. Is just seems out of place for entities. In my mind, entities simply transport data to & fro - not worry about updating the UI.
I have also been considering creating a property generator tool. Pop it off the menu, fill it out, and it adds a new property to your class. Would have options including RaisePropertyChanged etc.
If it's not broken, fix it until it is
|
|
|
|
|
Kevin Marois wrote: In my mind, entities simply transport data to & fro - not worry about updating the UI
This is the issue I am currently debating, the alternative seems to be to have yet another set of entities that extend the POCO transport objects with the UI functionality and additional properties.
You then need to translate between the 2 object types etc. etc. One of our senior devs has experience with EF4 and is recommending that and I know some here approve of it!
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
Nothing wrong with your BOs/DAL supporting INPC and INCC. That's why EF supports it out of the box. Even before EF, I always had my BOs and DALs implement INPC and INCC.
|
|
|
|
|
Well, that's on opinion. Not that it's wrong, but I'm still on the fence.
Again, INPC is a WPF thing. Suppose I do implement this in my Entities (and I AM leaning that way) and then a year from now the WPF UI is deprecated. For the ASP UI, INPC would neverbe used, and if the WPF app was shelved, then is really is not used. Do I then go back through all the entities and refacor them back to how they are now?
If so, then that means I would be making that change now to address a 'shortcoming' in WPF, and by definition, should find an alternative now.
Maybe I'm overthinking it, but again, it feels wrong somehow.
If it's not broken, fix it until it is
|
|
|
|
|
If you use a code generator, you'd just regenerate them. If you don't use a code generator, you would just change your "ObservableObject" base class. Granted, all your entities would still have the if (blah != value) RaisePropertyChangedEvent(), but that function wouldn't do anything. Heck, you could even do something clever like (and this is just psuedo-code to give you the idea):
public string SomeProp
{
get { return base.GetProp("SomeProp"); }
set { base.SetProp("SomeProp", value); }
}
obviously the base class will store a dictionary of string <-> objects, or whatever.
in base.SetProp() you would do something like (whilst supporting WPF) again psuedo code:
void SetProp(string propName, object value)
{
object o;
_dict.LookUpObject();
if ((o as IComparable).CompareTo(value) != 0)
{
_dict.UpdateValue()
RaisePropertyChanged(propName);
}
}
then if you remove WPF support down the road, you would just change this function and get rid of RaisePropertyChanged() and remove the INPC implementation.
|
|
|
|
|
Ok, i read this earlier, and then again now.
I like the idea, primarily becuase 1) only changed properties are updated, and 2) easy to take out.
While I get the concept, I could a bit of help with the code:
When you say "the base class will store a dictionary of string <-> objects, or whatever.", can you explain that a bit? Do you mean storing a dictionary of property names? Not sure I get how to code this.
If it's not broken, fix it until it is
|
|
|
|
|
Yes, a dictionary of propertyName <-> object. The setters will be a simple search and replace... but the getter, you'd have to typecase the result before return, so a prop would go from:
public string Caption { get; set; }
public string Caption
{
get { return (string)base.GetProp("Caption"); }
set { base.SetProp("Caption", value); }
}
the dictionary key will be Caption in this case and the value would be value. Sort of the same concept as dependency properties.
|
|
|
|
|
Alternatively, if you don't want to pollute this model, remember that the full pattern is called MVVM. The thing is, you are binding directly to your model here and that's not always the done thing. What I would do, if I were you, would be introduce a ViewModel that encapsulates the part of the AddressEntity that you are interested in.
There are a few reasons that I would do this, but one of the most basic ones is that your caption is unconstrained by the model. Now suppose that you want to guarantee that the user cannot submit a blank caption, then you simply put validation on your VM. As I don't know what your validation requirements are here, I would advise you to read up on validating data entries in MVVM.
|
|
|
|
|
Pete O'Hanlon wrote: What I would do, if I were you, would be introduce a ViewModel that encapsulates
the part of the AddressEntity that you are interested in.
I don't get it.
All that I have is a list of addresses. When Edit is clicked, the SelectedAddress is passed to another dialog for editing. When the dialog closes, I update the item in the list from the selected address (now updated and returned from the edit dialog).
The list is not refreshed. The model's have nothing to do with this, IMHO.
If it's not broken, fix it until it is
|
|
|
|
|
There is no “rule” that says each property must raise its own property changed event.
Here's a class where I used reflection to raise the event for all properties in a class after performing an update.
(I call "Notify" anytime I've updated some "stats"; I consider the overhead of calling the event for any properties that did not change insignificant).
using System;
using System.ComponentModel;
using System.Reflection;
namespace ModbusProtocol {
public class ComPortStats : INotifyPropertyChanged {
public event PropertyChangedEventHandler PropertyChanged;
private PropertyInfo[] _pInfo;
public int WriteRequests { get; set; }
public int Writes { get; set; }
public long BytesWritten { get; set; }
public int WriteErrors { get; set; }
public int ReadRequests { get; set; }
public int Reads { get; set; }
public long BytesRead { get; set; }
public int ReadNoReply { get; set; }
public int ReadTimeouts { get; set; }
public int ReadErrors { get; set; }
public int CRCReadErrors { get; set; }
public ComPortStats() {
Type objectType = this.GetType();
_pInfo = objectType.GetProperties(
BindingFlags.Public | BindingFlags.Instance );
}
public void Notify() {
foreach ( PropertyInfo p in _pInfo ) {
OnPropertyChanged( p.Name );
}
}
protected void OnPropertyChanged( string name ) {
if ( PropertyChanged != null ) {
PropertyChanged( this, new PropertyChangedEventArgs( name ) );
}
}
}
}
|
|
|
|
|
Very interesting idea. This code could go in my EntityBase. So then you just call Notify from the UI?
If it's not broken, fix it until it is
|
|
|
|
|
You can call Notify() from "anywhere" where Notify is visible for that instance ... even from a method within the class itself (as when doing "final totals", for example).
Just do the call when all updates are complete for that particular instance and the UI should reflect the changes.
|
|
|
|
|
Yes that code could/should live in your entity base, but calling it may be best done either when the edit address dialog is closed, or when you call context.SaveChanges() depending on when you want the update to perform. But typically there should be no need for the UI to ask the model to refresh, it should be the other way around, the data telling the UI that it has changed, this way when you use your address some where else you don't need to have another call to notify from the UI to the Model.
|
|
|
|
|
If you want to update all properties, the easier way to do this is just to call PropertyChanged with no property name in the PropertyChangedEventArgs . That updates all of the properties. BTW, you have a possible race condition in your OnPropertyChanged method. Do this instead:
protected void OnPropertyChanged(string name)
{
PropertyChangedEventHandler handler = PropertyChanged;
if (handler != null)
{
handler(this, new PropertyChangedEventArgs(name));
}
}
|
|
|
|
|
|
What's a 'race condition'?
If it's not broken, fix it until it is
|
|
|
|
|
Multiple threads potentially hitting the same piece of code around the same time. In the original example, there is a test for a null handler, and if it's not null, the event is raised. The race condition occurs because these are two discrete operations so the second call to the handler could be null because another thread freed the event handler. That's what my code fixes (there are other ways, but this one's a common way).
|
|
|
|
|