Best practice for complex shared/static members

I took over an ASP.NET application and have found this throughout several classes in the application. The programmers before defined several shared/static variables that act as "complex enums" throughout the application. As a fairly new programmer, it doesn't look like best practice.

Here is an example:

Public Shared SecureCommentsWrite As New Task("Secure Comments Write")
Public Shared SecureCommentsRead As New Task("Secure Comments Read")
Public Shared EditEmergencyContact As New Task("Edit Emergency Contact")
Public Shared DisplayPersonalReferences As New Task("Display Personal References")
Public Shared EditPersonalReferences As New Task("Edit Personal References")

The constructor takes the description, then loads the ID key from the database using a stored procedure (the database is SQL Server.) This seems like a good idea since we deploy this application to multiple databases and want to ensure that we load the ID key that's in that database in case it changes. However, since there are literally hundreds of these in the application, the first load takes a while.

Is this considered best prac开发者_高级运维tice, and if not, what is considered to be best practice for a situation like this?

For me this is an horrible practice, if you are telling me that for each of those rows above and Task constructor a separated stored procedure is called (even if same stored).

best practice in this case would be to refactor, make a single call to that stored procedure and modify it to return all IDs and Names then having a little TaskManager or TaskLoader (whatever) class which maps the stored's results and creates all those elements with no further database involvement.

in these cases notice that a SqlDataReader would probably be better than a DataSet because you just need forward only, read only fast access to the data.

that's all for now from my side ;-)

There's nothing inherently wrong with having lists of static fields that represent constant values; it is, as you pointed out, essentially the same as an enumeration, and Microsoft has done it themselves in some of their own libraries.

That said, if initializing these fields is causing a noticeable slowdown (and since they're each hitting a database, that's no real surprise), there are a few techniques you could use to improve loading times. An obvious solution is to employ lazy loading--in other words, don't hit the database until you absolutely have to! This essentially amortizes the cost of hitting the database to initialize these fields over the entire life of the program, giving you a much faster startup in exchange for somewhat slower performance elsewhere.

Of course, if you have to lazy load 100 or 1000 of these at once, that may not be the ideal solution, either; in that case you've just moved the massive delay around, rather than breaking it up.

Another idea would be to improve the efficiency of the SQL responsible for retrieving these IDs. You might write a single Initialize() method that performs a single query to pull out all of the IDs you need at once, rather than doing it in 100 different queries. That will almost certainly be faster.

You probably want a factory class here (TaskFactory?).

It should load (once) a DataTable or DataSet that represents the entire list of Task items. This could be done by the constructor, or [lazily] when the first request for an instance is executed.

Rather than hit the db each time, the factory's CreateInstance() method then should consult the preloaded data for what it needs.

It is not a good practice to run database queries / stored procedures in object constructor, or any other possibly time-consuming operations. If something is wrong during initialization of a Task object, since it's declared as a static/shared member it will possibly throw a TypeInitializationException and your application will become unusable.

I would treat these members as some kind of application configuration. Execute a stored procedure during Application_Start which will bring all the data at once (and not one by one), and create the configuration object(s).