|
Hi,
It is good in case of we have to load one object( eg Person ) at a time . What if we have to load 2 or more object at same time like below.
Class Address{
}
Class Person
{
private Address _address;
}
suraj
|
|
|
|
|
Hello,
You could do it in the Person constructor:
public class Person : PersistentDataObject
{
public Person() : base("Person", "PersonId") { }
public Person(int id) : this()
{
Load(id);
address = new Address(AddressId);
}
private AddressId { get { Get<int>("AddressId"); } }
private Address address;
public Address Address { get { return address; } }
}
I usually instantiate composite objects lazily, like so:
public class Person : PersistentDataObject
{
public Person() : base("Person", "PersonId") { }
public Person(int id) : this() { Load(id); }
private AddressId { get { Get<int>("AddressId"); } }
private Address address;
public Address Address
{
get
{
if(address == null)
address = new Address(AddressId);
return address;
}
}
}
Nick
|
|
|
|
|
hi, it's nice to add some sample to your code to make it Database Independent!
It's very usefull to access other DBMS like MSAccess, mySql, SqLite, etc...
|
|
|
|
|
Is there any convenient way to bind Person object illustrated herein to a View object as GridView.
Regards,
Rciky.
|
|
|
|
|
I just bind a List of the objects the same way I did with the Repeater in the demo code. For example I might have replaced the Repeater with this GridView:
<asp:GridView runat="server" ID="GvPeople" AutoGenerateColumns="false">
<Columns>
<asp:TemplateField>
<ItemTemplate>
<%#Eval("FirstName") + " " + Eval("LastName")%>
</ItemTemplate>
</asp:TemplateField>
<asp:BoundField DataField="DateOfBirth" HeaderText="Date of Birth"
DataFormatString="{0:d}" HtmlEncode="false" />
<asp:BoundField DataField="LuckyNumber" HeaderText="Lucky Number" />
</Columns>
</asp:GridView>
N
|
|
|
|
|
Hi man! I have a question. How you retrieve the primary key generated automaticaly? Example:
Person person = new Person();
person.FirstName = "Nick";
Whats the people.ID in this context? After insert a record, I need to save a picture, per example. How save the file as "ID + _picture.jpg"? In my persistence object, I create a method to reload the record after the Insert, reloading the last record inserted as:
Select(String.Format("{0} in (select ident_current('{1}'))", primaryKeyFields[0], nameTable));
Think about this, ok?!
And, you can create new "LOAD" methods to increase you class and create a code more fast, adding:
public void Select(string where)
public void Select(string where, string orderby)
public void Select(string campos, string where, string orderby)
public void SelectByPrimaryKey(int primaryKeyValue)
public void SelectByPrimaryKey(string primaryKeyValue)
public void SelectByPrimaryKey(params int[] primaryKeyValues)
public void SelectTop(int numberOfRegs)
public void SelectTop(int numberOfRegs, string where)
public void SelectTop(int numberOfRegs, string where, string orderby)
public void SelectByQuery(string query)
(...)
I´will change my Get and Set methods for you solution. Very well!
Felipe Drumond, from Brazil
|
|
|
|
|
Hi felipedr, thanks for the feedback.
felipedr wrote: Person person = new Person();
person.FirstName = "Nick";
Whats the people.ID in this context? After insert a record, I need to save a picture, per example. How save the file as "ID + _picture.jpg"?
If an instance of a PersistentDataObject has not yet been inserted into the database, as in your example, the Id property contains the value -1. However, when Save is called the Id is automatically set to the primary key value of the object in the database. So, the Id is available immediately after the Save method is called.
felipedr wrote: In my persistence object, I create a method to reload the record after the Insert, reloading the last record inserted as:
Select(String.Format("{0} in (select ident_current('{1}'))", primaryKeyFields[0], nameTable));
Think about this, ok?!
I was using @@IDENTITY to retrieve the Id, but after seeing your example and looking up IDENT_CURRENT and SCOPE_IDENTITY I see that that SCOPE_IDENTITY is a better method, and I’ll be changing my code to use that instead.
Note that you can do the INSERT query and the SELECT query in a single command, so you don’t have to do an extra round trip to the database server, like in this snippet from PersistentDataObject:
string sql = "SET NOCOUNT ON;"
+ "INSERT "+tableName+" ("+fieldList+")"
+ " VALUES ("+valueList+");"
+ " SELECT SCOPE_IDENTITY()"
+ " AS InsertionId;";
sc.CommandText = sql;
idValue = Convert.ToInt16(DatabaseAccessor.ExecuteScalar(sc));
felipedr wrote: And, you can create new "LOAD" methods to increase you class and create a code more fast, adding:
public void Select(string where)
public void Select(string where, string orderby)
public void Select(string campos, string where, string orderby)
I agree that a quick intuitive way to get collections of data objects is the next logical step up, and I appreciate the suggestion. I will be doing some work on this in the future and if I come up with something worth sharing I'll post it up.
Thanks,
Nick
|
|
|
|
|
Hi,
thanks for sharing experiences.
I was wondering if you've had any time to think about pitfalls or how-to's in respect to managing transactions
(commits and rollbacks) in regard to your sample design?
Cheers,
- P
|
|
|
|
|
Thanks probe53,
Transactions management is one of directions in which I want to develop this. I haven't had the opportunity to get into this in any great detail yet, but I do have some idea what needs to be done.
Firstly, of course DatabaseAccessor would need methods adding to begin, commit, and rollback transactions. Then all PersistentDataObject instances participating in a transaction would need to execute their queries using the same SqlConnection. Therefore, they’ll all need to use the same instance of DatabaseAccessor. This could be achieved by implementing DatabaseAccessor as a singleton.
Finally, a single method should be responsible for began and committing (or rolling back) any one transaction.
I would expect (and recommend) all that dependant data objects will be combined via composition, so that whenever you want to perform a transaction you will do so by calling either the Save or Delete method of an instance of PersistentDataObject.
For an example, say that in addition to the Person class in my sample, we also have an Address class that is also a subclass of PersistentDataObject. Say we want a person to have an address associated with it. An appropriate way to implement this would be though composition, so we might add something like this to the Person class:
private int AddressId
{
get{return Get<int>("AddressId");}
set{Set("AddressId", value);}
}
private Address address;
public Address Address
{
get
{
if(address == null)
{
if(Id == -1)
Address = new Address();
else
Address = new Address(AddressId);
}
return address;
}
}
Then we'd want to save changes to the address when we save changes to the Person, so we'd override the Save of Person method with something like this:
protected override void Save()
{
Address.Save();
AddressId = Address.Id;
base.Save();
}
Now let's imagine we've made the modifications to DatabaseAccessor that I mentioned above; we might wrap the Address and Person save calls in a transaction by modifying the Save method as follows:
protected override void Save()
{
DatabaseAccessor.BeginTransation();
Try
{
Address.Save();
AddressId = Address.Id;
base.Save();
DatabaseAccessor.CommitTransation();
}
catch(Exception e)
{
DatabaseAccessor.RollbackTransation();
}
}
Obviously there's still work to do, but that's a rough idea of how I think it'll work.
What do you think?
Nick
|
|
|
|
|
I think too many people look at these starter articles and expect every problem in the universe to be solved.
I always like to see other ideas/implementations of ORM to see if there is anything I would like to add to my own meager endeavors. I built my implementation before generics(even though Microsoft's implementation leaves me cold), so I am looking forward to a small rewrite in the near future.
SQL injection problems are easy to add to your solution, and the connection string thing was probably done that way to get operational code up and running. most of us have solutions to all of those problems already in our tool box.
I am glad that others point these things out as they help new users understand where the mines are.
However, overall, this is a very good article, and a good start for those waiting for MS to ship/deliver a standard we can all rally around.
|
|
|
|
|
Thanks for the positive words Dewy,
I certainly wouldn't expect the approach presented in the article to be suitable in all situations, but I think it would be useful in a high proportion of them, and could form a solid basis for more substantial frameworks.
By the way, as I mentioned in my response to reinux, if you look closely at the
PersistentDataObject code, it is entirely safe from SQL-Injection attacks.
Nick
|
|
|
|
|
1. Hard coded connection strings: Your connection string's there as a constant in your source. I hope that password you have there isn't actually your password.
It'd be a better idea to let the user set the SqlConnection object. DatabaseAccessor is probably unnecessary too; it'd be a better idea to put all that functionality inside PersistentDataObject itself.
2. SQL injection: If someone had a string value of something like: "'); DELETE FROM Persons;", you're screwed. Use SqlQueryBuilder so that everything is escaped properly.
|
|
|
|
|
Thanks reinux,
I would indeed expect the connection string read from a config file when this code is used. I hard coded it in for the this article for simplicities sake.
In PersistentDataObjcet all values added to SQL queries are done so via parameters, so the class is safe from SQL-injection attack.
Nick
|
|
|
|
|
reinux wrote: Not to beat a dead horse or anything, but I would have been less inclined to question the security of your code had you explained in the article or code why you had a dummy connection string with a dummy password, which you later explained that you expect none of your readers to follow such example. Think of the beginners who might have gone ahead and done the same thing!
Are you suggesting that impressionable beginners who see that I have posted code containing a connection and string may "follow my example" by posting code containing a connection string that could be used to connect to a database via the internet? Well, I guess it could happen, but it's not a flaw in the code its self though is it? Hard coding a connection string is considered bad practice because it doesn't promote portability, that is, you can't swap to another database without recompiling the code. It doesn’t make the application any less secure.
reinux wrote: It'd be a better idea to let the user set the SqlConnection object. DatabaseAccessor is probably unnecessary too; it'd be a better idea to put all that functionality inside PersistentDataObject itself.
DatabaseAccessor contains functionality that is likely to be needed by classes other than PersistentDataObject, so it makes complete sense to encaspulat it in a separate class. Even if this wasn't the case, it's good practice for a class not to have disparet functionality.
I included DatabaseAccessor for completeness, but I would expect that most developers will already have a similar class.
DatabaseAccessor contains functionality that is likely to be needed by classes other than PersistentDataObject, so it makes complete sense to encapsulate it in a separate class. Even if this wasn't the case, it's good practice for a class not to have disparate responsibilities.
I included DatabaseAccessor for completeness, but I would expect that most C# developers will already have a similar class that they use most of the time.
|
|
|
|
|
Nick Higgs wrote: Are you suggesting that impressionable beginners who see that I have posted code containing a connection and string may "follow my example" by posting code containing a connection string that could be used to connect to a database via the internet? Well, I guess it could happen, but it's not a flaw in the code its self though is it? Hard coding a connection string is considered bad practice because it doesn't promote portability, that is, you can't swap to another database without recompiling the code. It doesn’t make the application any less secure.
It increases the risk of human error.
You may be more inclined to share your code and your connection string along with it, whereas you'd think twice about sharing a .config file written in plain xml.
This is what I thought you might have done at first, even though it seemed almost too silly, that you just carelessly cut & pasted code from your workplace code and shared it on CP. It does happen.
Is it a flaw in the code to neglect setting good examples for beginners? Arguably not, but it is certainly a flaw in the article.
Nick Higgs wrote: DatabaseAccessor contains functionality that is likely to be needed by classes other than PersistentDataObject, so it makes complete sense to encapsulate it in a separate class. Even if this wasn't the case, it's good practice for a class not to have disparate responsibilities.
There's a limit.
You don't want to expose anything from your class library that doesn't immediate accomplish what the library is designed for, unless it's a Helper class of some sort that makes some sophisticated manoeveurs or provides validated access to private features. Clearly, DatabaseAccessor doesn't.
Nick Higgs wrote: I included DatabaseAccessor for completeness, but I would expect that most C# developers will already have a similar class that they use most of the time.
Yet PersistentDataObject is reliant on DatabaseAccessor.
Let's examine the try{} blocks of the methods in this class.
ExecuteQuery:
sqlCommand.Connection = SqlConnection;
SqlConnection.Open();
using (SqlDataAdapter sqlDataAdapter = new SqlDataAdapter(sqlCommand))
sqlDataAdapter.Fill(ds);
ExecuteScalar:
SqlConnection.Open();
sc.Connection = SqlConnection;
obj = sc.ExecuteScalar();
ExecuteNonQuery:
SqlConnection.Open();
sqlCommand.Connection = SqlConnection;
iRowsAffected = sqlCommand.ExecuteNonQuery();
Considering that the calls to DatabaseAccessor are also wrapped in try/catch blocks, you've effectively only isolated maybe 10 lines of code, all of which is very standard stuff. You say that you expect most c# developers would already have similar classes for this (I wouldn't have a class just for this by the way), which would render PersistentDataObject unusable without modification.
|
|
|
|
|
The article is intended to demonstrate the approach that I take to persistent object management. Working code is provided to this end. As persistent object management is a fundamental part of most software, most developers will already have pretty well developed approaches that fit in with the way they work. Therefore, I thought it appropriate to minimize that the coverage of the example code so interested developers can understand how it works and fit it into their existing approach as easily as possible.
reinux wrote: You don't want to expose anything from your class library that doesn't immediate accomplish what the library is designed for...
As mentioned above, I've presented the code as a demonstration of an approach, rather than a class library.
reinux wrote: ...PersistentDataObject is reliant on DatabaseAccessor ... which would render PersistentDataObject unusable without modification.
The only reason I provided DatabaseAccessor was so the code would work without modification, and it does. My point was that many developers would want to switch it for a similar class that they already use.
As you started this thread to point out security flaws (that, as it turns out, don't exist) and you're now focusing on the division and exposure of responsibilities, I think you're nit picking. The criticisms you've made are subjective and don't indicate any bad practice in my code; therefore, I'm going to consider this thread concluded.
Thanks,
Nick
|
|
|
|
|
Nick Higgs wrote: The only reason I provided DatabaseAccessor was so the code would work without modification, and it does. My point was that many developers would want to switch it for a similar class that they already use.
As you started this thread to point out security flaws (that, as it turns out, don't exist) and you're now focusing on the division and exposure of responsibilities, I think you're nit picking. The criticisms you've made are subjective and don't indicate any bad practice in my code; therefore, I'm going to consider this thread concluded.
Actually I started this thread with the security flaws (which turned out not to be) and mention of DatabaseAccessor, the latter being which we've been talking about the whole time.
I don't think it's subjective the fact that most people already have their own ways of connecting to their databases (ie the normal way), thus most people will have to modify your code to make it useful. Sometimes you say it's intended to work without modification, sometimes you say it's just a demonstration.
Here's what I would have wanted to see in particular: in either the PersistentDataObject class constructor or method calls, have as a parameter a SqlConnection object. Then in each of the Load/Save/Delete methods call SqlConnection.Open(), do the query in a try-finally block using a SqlDataAdapter generated in the method. That way you don't need DatabaseAccessor nor DatabaseAccessException, and the code works anywhere as is.
On the other hand I'll agree that the other thread on stored procs and OOP is more subjective, although I'm still not convinced that stored procedures for CRUD is either "silly" or "dangerous".
So I'll just end it at this to clarify my reasoning:
Nick Higgs wrote: I'm not sure what your point is, surely you're not suggesting that a stored procedure based business tier is "procedureless". My point was that a set of stored procedures have no structure connecting them or defining them relative to each other, which means they don't assist much when trying to maintain data integrity.
My point is that while relational databases provide a clean way to structure information, they present themselves as a jumble of moving parts for which you're responsible for understanding fully before being able to use. Where do you insert data? Which columns can you update without messing up integrity? Are there more columns from elsewhere that I need to pull if I need to do a calculation? I personally would prefer a set of verb-noun operations that work as I expect them to, rather than a whole bunch of nouns for which I (or worse, someone else) must carefully define the verbs.
|
|
|
|
|
Why do you feel stored procs are silly to use for simple CRUD procedures? Aside from compartmentalizing your SQL into tidy packages, procs also allow you to refrain from having dynamic SQL in your app, enforce transaction management (which I feel should be done in the database for DMO), and simplify security.
I'm not going to try and argue the point with regards to compiled execution plans, because for simple queries the optimizer is remarkably efficient at JIT plan creation, and if a proc is faster for simple statements it would only be marginally so IMHO.
|
|
|
|
|
Hi chmod755,
Thanks for responding to my article.
The reason I added that (admittedly provocative) statement to the article, is that an objection I've encountered to the approach outlined in the article is that it doesn't allow the use of stored procedures. As you're clearly aware the Stored Procedures vs. Dynamic SQL debate is too large to rehearse in full here, but the reason I think using stored procedures for simple CRUD queries is silly is that it creates a substantial amount of additional development overhead without introducing any actual benefit.
I don't find the potential benefits you mentioned compelling, and I'll explain why briefly. Firstly, I don't regard stored procedures as "tidy packages", rather I find them awkward and brittle. You mentioned that stored procedures "allow you to refrain from having dynamic SQL in your app"; I would argue that, if properly encapsulated, dynamic SQL is not harmful. With regard to your final two points, both transaction management and finely grained security can be achieved by other means, and besides, transaction management is not relevant when executing a single CRUD query.
I believe that stored procedures are often used simply because that's "the way it's done". Put another way, I find stored procedures often become an end in themselves, instead of a means to an end.
Thanks again for your response,
Nick
|
|
|
|
|
IMHO, the arguement for using stored procedures is mainly for security reasons. For example, in my applications no users have access to insert, update, or delete data from any of the tables. All the writing or deleting of data is done through procedure. This has worked out very nicely for me since I can give some super users of my applications direct access to the query the tables (using Access) without having to worry about them deleting or modifying any data in the database. This saves me from having to pull an endless number of reports. If I was using dynamic SQL to I would have no good way of locking down the tables.
Mike Lasseter
|
|
|
|
|
Pretty and tidy are two different things. Stored procs aren't pretty but they are tidy. Data integrity and business logic should be done as close to the data as possible. So I use stored procs regardless even though I don't like T-SQL.
If a few months down the line you need to write another client application, you don't want to have to dig up your old app and guess your way through the logic. It leaves a lot of room for error, and if someone else who doesn't have access to your client application wants to do something with the database, they won't know how to use it properly.
Read up on the Model-View-Controller pattern[^] and you'll see the reasoning behind it.
|
|
|
|
|
The distinction between pretty and tidy notwithstanding, if you're trying to explain the importance of the separation of business logic and application logic, I agree with you, and I would have thought it was clear from my article that I'm well acquainted with such ideas. However I would question the suitability of stored procedures as a technology for the business tier. It seems odd that you're advocating separating business and application logic by tying business and data logic closer together.
|
|
|
|
|
Nick Higgs wrote: However I would question the suitability of stored procedures as a technology for the business tier. It seems odd that you're advocating separating business and application logic by tying business and data logic closer together.
Why?
|
|
|
|
|
reinux wrote: Why?
Hello again reinux,
I'm delighted that you've taken an interest in my comments, but I'm not willing to continue this tangential discussion without meaningful participation. Your original post included this statement:
reinux wrote: Data integrity and business logic should be done as close to the data as possible.
This is the point I'm focusing on because it has a bearing on the approach I've outlined in my article. In my previous post I intended to point out that the above statement is contrary to the separation of interests you seem to be advocating, and advocates the use of T-SQL for a purpose it was not designed for. With all this in mind I would be very interested to hear why you believe this.
Thanks,
Nick
|
|
|
|
|
Sorry, I didn't mean to be excessively terse but I genuinely don't understand:
1. why you don't see stored procedures being unsuitable for the business tier.
2. why it seems odd to you that I'm advocating separation of business and application logic
And now I don't understand:
3. why "keeping data integrity and business logic close to the data is contrary" to
4. which "separation of interests that I'm advocating".
As for T-SQL being used for a purpose that it wasn't designed for, bear in mind that functions in C were at first not intended to be used to assign/retrieve values to variables, until C++ and the OOP paradigm became popular and it was decided that it is in fact important for data objects to be fully autonomous and guarantee valid data at all times at least on the executing thread.
Given a list of tables, can you guess which ones you can insert data into without breaking data integrity? On the other hand, if you were given a list of stored procs, given they are named meaningfully, you would know that these are the interfaces that the database was (surprise) designed to be used with. Ensuring proper use of the database outweighs abiding to intended uses of tools.
Your application will not necessarily be provided with the database, so if your application is non-existent to the next person who wants to program against the database, that person is in trouble. And even if it does exist, they have to rummage through your code and guess what special steps you've taken to ensure that the data is valid.
One thing about The Code Project is that everyone likes to hear the whys as much as the hows.
Not to beat a dead horse or anything, but I would have been less inclined to question the security of your code had you explained in the article or code why you had a dummy connection string with a dummy password, which you later explained that you expect none of your readers to follow such example. Think of the beginners who might have gone ahead and done the same thing!
|
|
|
|
|