Is this correct OO design?

  • Thread starter Thread starter cmo63126
  • Start date Start date
C

cmo63126

I'm not sure if this is bad design or not. It seems flawed, but I'm not
sure. Is it wrong to create an instance of a class within the class
itself? Any feedback appreciated.

public class Article
{
int articleID;
string title;

//Constructor: Load Data from DataAccess Class
public Article(int articleID)
{
this.articleID = articleID;

//Set properties of Article Object with values from Database
DataAccess.GetArticleData(this, articleID);
}

public void Update()
{
/*
IS IT BAD OO DESIGN TO CREATE AN INSTANCE OF A CLASS
(OriginalArticle) WITHIN THE CLASS ITSELF(Article)?
*/

Article OriginalArticle = new Article(this.articleID);

//check to see if the new and origianal values match
if(this.title != OriginalArticle.Title)
{
Response.Write("<br>Title doesn't match current Title in
Database...Perform Important Operation Here.");
}
else
{
Response.Write("<br>Title hasn't changed.");
}
}

public string Title
{
get{return title;}
set{title = value;}
}
}

public class DataAccess
{
public static void GetArticleData(Article article, int ArticleID)
{
//load database data, populate Article object from parameter
article.Title = "Original Title from Database";
//article.Field1 = ...
//article.Field2 = ...
//etc...
}
}

public class _Default : Page
{
private void Page_Load(object sender, EventArgs e)
{
Article MyArticle = new Article(2112);
Response.Write("<br>MyArticle.Title: "+MyArticle.Title);
MyArticle.Title = "New Title from MyArticle object instance";
MyArticle.Update();
}
}

results:
MyArticle.Title: Original Title from Database
Title doesn't match current Title in Database...Perform Important
Operation Here.
 
This is NOT bad design. It is not particularly efficient, but there is no
magical OO principle that is violated.

In fact, optimistic concurrency mechanisms often do something similar
(usually in the DB layer) by reading the matching db record first, before
updating the row, to check if the db value is different from an expected
value, either in all of the columns, or in a timestamp column.

Is that what you are trying to build logic for? A condition where two
people have opened a record, and modified it, and one has saved their
changes (to the title in your example)? If so, you may want to take a close
look at the timestamp data type in SQL Server, and Optimistic Concurrency
with Row Versioning.

This link may help.
http://msdn.microsoft.com/library/d...s/cpguide/html/cpconOptimisticConcurrency.asp


--
--- Nick Malik [Microsoft]
MCSD, CFPS, Certified Scrummaster
http://blogs.msdn.com/nickmalik

Disclaimer: Opinions expressed in this forum are my own, and not
representative of my employer.
I do not answer questions on behalf of my employer. I'm just a
programmer helping programmers.
--
 
No it's not a concurrency-related issue. I have a news portion of my
web app. where each news article has the potential for a document
library associated with it. if the user creates a press release and
wants to upload related documentation, they have the ability to do so.

there's 3 areas that potentially must be modified when an article has
changed:

1. Articles Table: housing all data related to article itself
Fields include: (title, summary, body, opendate, closedate,
DepartmentName, DocLibraryID (see 2.), etc...

2. DocLibrary Table: housing all documents (if any) associated with a
given article.
Fields include: (ArticleID, FilePath, other irrelevant fields)

3. Physical Directory where docs are stored.

when the article is updated - I assuming it's the proper way of doing
so - I start my changes from the bottom up in steps 3-2-1 fashion.

I my example I need to check if the user has modified the article to
display under a different department or has changed its display date.
This will effect the physical path to the article's document library.

for example:
Original article: Created for "Public Relations" on January 31, 2005
path: /Docs/News/PublicRelations/2005_01_31/

Updated article: Moved to "General News" on April 15, 2005
new path: /Docs/News/GeneralNews/2005_04_15/
 
Hello cmo63126,

In early days of database development, it was common for the primary key of
a record to be derived from the values in the record. So, for example, the
ID of a customer John Smith who lives on 123 Main St may be JOSMI12X999 with
the last four characters used to create a unique id between this record an
the record of Joe Smart at 123 Elm.

It quickly became apparent that there's a problem with this: What happens
if John Smith moves from 123 Main to 765 Willow? Do you change his ID? and
doesn't that mean that you have to find all the records linked to his ID and
change them too?

You have created the EXACT same problem for yourself. You have tied the
location of the document to a variable value in a database record, which
means if the value changes, you have to change the location... and you are
paying for it by creating a needlessly complex design to cope.

Strong suggestion:
Do not place files in a fileshare that has anything at all to do with the
department. Create a tool for placing files in the site doc library.
First, get a unique and arbitrary value for the id of your document. (if it
is random, like a GUID, you can do interesting things like create
subdirectories based on parts of the ID). For simplicity sake, let's say
you put 100% of you files in a single "documents" folder on your site. Your
tool will rename the file to the ID (keep the extension), and place it into
the Documents folder. In your documents table, put in metadata, like the
original file name, date imported, and the user id of the document owner
(assuming you want to eventually find the document, or delete it, or replace
it with a new version).

Now, when the user changes the department of the news item, your document
doesn't move. You also get the advantage that a document cannot easily
overwrite another document because the name happens to be the same (common
in sales or marketing organizations, where most documents start with a
template).

--
--- Nick Malik [Microsoft]
MCSD, CFPS, Certified Scrummaster
http://blogs.msdn.com/nickmalik

Disclaimer: Opinions expressed in this forum are my own, and not
representative of my employer.
I do not answer questions on behalf of my employer. I'm just a
programmer helping programmers.
--
 
Back
Top