Sunday, March 27, 2011

Class Library of Constants--Best Practice?

I was using .Net Reflector on an Internal App to try and understand what the previous Dev was doing and also to learn. I have never had actual instruction on how to develop Apps so I take from where I can (Hooray Stack Overflow). That being said I found something that has me confused. A class Library called WinConstant containing the below code.

Here are my actual question:

  1. What possible use could this be?

  2. What value is there in storing a bunch of constant's in a class library?

  3. Is this considered a "Best Practice"?

Thoughts and guidance appreciated!


Public Class clsConstant
    Public Const cAccess As String = "Access"
    Public Const cAddress As String = "Address"
    Public Const cCancel As String = "Cancel"
    Public Const cCity As String = "City"
    Public Const cClear As String = "Clear"
    Public Const cClickOnMessage As String = "Click on any row in top pane to see the detail fields in the bottom pane."
    Public Const cClientID As String = "ClientID"
    Public Const cColon As String = ": "
    Public Const cComma As String = ","
    Public Const cContactID As String = "ContactID"
    Public Const cCounty As String = "County"
    Public Const cDash As String = "-"
    Public Const cDelete As String = "Delete"
    Public Const cDepartment As String = "Department"
    Public Const cError As String = "Error"
    Public Const cExec As String = "Exec"
    Public Const cFalse As String = "False"
    Public Const cFavorite As String = "Favorite"
    Public Const cFederal As String = "Federal"
    Public Const cFriday As String = "Friday"
    Public Const cfrmMain As String = "frmMain"
    Public Const cfrmModuleLogin As String = "frmModuleLogin"
    Public Const cfrmModuleSplash As String = "frmModuleSplash"
    Public Const cHelp As String = "Help"
    Public Const cHint As String = "Hint"
    Public Const cImagePath As String = "../../image"
    Public Const cIn As String = "In"
    Public Const cInformation As String = "Information"
    Public Const cInitialScreenID As String = "InitialScreenID"
    Public Const cInsert As String = "Insert"
    Public Const cJuvenileID As String = "JuvenileID"
    Public Const cLetter As String = "Letter"
    Public Const cManual As String = "Manual"
    Public Const cMasterID As String = "MasterID"
    Public Const cModuleID As String = "ModuleID"
    Public Const cModuleName As String = "ModuleName"
    Public Const cMonday As String = "Monday"
    Public Const cName As String = "Name"
    Public Const cNegative As String = "Negative"
     _
    Public Shared ReadOnly cNLowDate As DateTime = New DateTime(&H851055320574000)
     _
    Public Shared ReadOnly cNullDate As DateTime = New DateTime
    Public Const cNullDateString As String = "12:00:00 AM"
    Public Const cOfficeIDDefault As String = "01"
    Public Const cOne As Integer = 1
    Public Const cOut As String = "Out"
    Public Const cPopUp As String = "PopUp"
    Public Const cPositive As String = "Positive"
    Public Const cProcess As String = "Process"
    Public Const cProviderID As String = "ProviderID"
    Public Const cQuestion As String = "Question"
    Public Const cRead As String = "Read"
    Public Const cReferralID As String = "ReferralID"
    Public Const cReminder As String = "Reminder"
    Public Const cReport As String = "Report"
    Public Const cReportEngine As String = "ReportEngine"
    Public Const cReportEnginePath As String = "ReportEnginePath"
    Public Const cReportingServices As String = "ReportingServices"
    Public Const cReportServer As String = "ReportServer"
    Public Const cReportService As String = "ReportService"
    Public Const cReportServiceLocal As String = "ReportServiceLocal"
    Public Const cReportServiceServer As String = "ReportServiceServer"
    Public Const cSaturday As String = "Saturday"
    Public Const cSearch As String = "Search"
    Public Const cSelect As String = "Select"
    Public Const cSpace As String = " "
    Public Const cSQLLoginError As String = "SQL Server login/password invalid"
    Public Const cStart As String = "Select a module"
    Public Const cState As String = "State"
    Public Const cSubjectID As String = "SubjectID"
    Public Const cSunday As String = "Sunday"
    Public Const cThursday As String = "Thursday"
    Public Const cTooltipCancel As String = "Reset form data values back to before all manual changes."
    Public Const cTooltipClear As String = "Clears all data entry fields prior to an Insert"
    Public Const cTooltipClient As String = "Display a Client popup window."
    Public Const cTooltipClose As String = "Close this form"
    Public Const cTooltipDelete As String = "Delete the current record being displayed, no undo possible."
    Public Const cTooltipExe As String = "Initiate a batch process."
    Public Const cTooltipInsert As String = "Insert a brand new record"
    Public Const cTooltipSearch As String = "Perform a Search for values entered."
    Public Const cTooltipSelect As String = "Perform a Select for values entered."
    Public Const cTooltipUpdate As String = "Update an existing record"
    Public Const cTrue As String = "True"
    Public Const cTuesday As String = "Tuesday"
    Public Const cUnderscore As String = "____________________________________________________________"
    Public Const cUpdate As String = "Update"
    Public Const cWarning As String = "Warning"
    Public Const cWeb As String = "Web"
    Public Const cWednesday As String = "Wednesday"
    Public Const cWorkerID As String = "WorkerID"
    Public Const cZero As Integer = 0
    Public Shared strLongDate As String() = DateAndTime.Now.ToLongDateString.Split(New Char() { ","c })
    Public Shared strModuleMainStatusStripFormID As String = Nothing
End Class
From stackoverflow
  • The only explanation I can think of is as some sort of message library - but that doesn't hold for 90% of the entries.

    For instance this is just silly:

     Public Const cInsert As String = "Insert"
    

    This smells very very bad.

  • A constant is something that never changes, for example

    Public Const NumberOne as Int = 1
    

    So this is my first remark: some of the stuff you summed up isn't really const.

    Another downside is that using the const keyword creates a binary depency. This means that you will have to rebuild the assembly which uses your constants.dll. You can't just replace it. This is caused by the way consts work: the compiler replaces the name with the value at compile time.

    A solution to this problem is to use ReadOnly instead of Const.

    I don't really think it is a good practice to create such a library. I wouldn't allow my team to create one anyway...

  • Separating out literals from the rest of the code is a good idea.

    What's odd is that these should largely be resources rather than constant strings. Then they could be easily localized if needed, or replaced/updated without a recompile of the entire app.

    Some of those shoudln't even be resources: cUnderscore for example looks like it's using text to create a visual effect- generally a bad idea.

    In your predecessor's defense, I consider this code preferable to finding the same constants scattered throughout the source, as it will make refactoring to resources a little simpler.

  • This looks like a developer had a coding standard that says: Don't use string literals in code, and dutifully separated out every single constant, whether it made sense or not.

    For example, there is probably some element in there where the number 1 was needed in the code and instead of using DefaultNumberOfLineItems or some other descriptive constant, they used NumberOne = 1;

    A best practice would be to keep constants descriptive and close to their point of use. There's nothing wrong with a static class of related constants that have some type of meaning and are related to each other.

    For example, there is a system that I've worked on that tags attributes with unique keys. These keys are gathered in a static class with descriptive names on the attributes, the actually keys are generated by an automated system

    public static class AttributeIDs
    {
       public const string Name = "UniqueGeneratedKeyForNameAttribute";
       public const string Description ="UnqiueGeneratedKeyForDescriptionAttribute";
       ... etc.
    }
    

    In practice this is used like

    MyAccess.GetValueForAttribute(AttributeIDs.Name);
    

    which gets all the related constants together.

  • There are possible useful scenarios for such a class. Generally, things like "magic number" or "magic strings" are turned in to constants and placed in a static (shared) class. The reason for this is to isolate those "magic" values to a single location and allow them to be referenced by a meaningful name. (Typically used for numeric values.) In the case of string values, it helps ensure that things are referenced by the same value each time. The best example of this is string keys in to your app.config file.

    That being said, constants should be used for something that doesn't change (or so rarely changes that it is effectively constant). For the most part, strings that have the potential to change (or need to be localized) should be stored as resources in a .resx file.

    Joel Coehoorn : resources > constants, and if anything easier
    Scott Dorman : @Joel Coehoorn: I think it depends on how they are being used. For instance, ensuring a consistent name for a bunch of DllImport attributes. I can't use a resource for the name of the DLL but I can use a constant.
  • There is nothing wrong with having a class library of constants. Constants are a good practice in general. Enums are all over the place in .NET after all, and they are just grouped numeric constants. Having a dependency on an assembly of constants is no different than any other dependency. Understanding the purpose of the constants is more about the app's logic. My guess is that these constants enable verbose logging without filling the app with a bunch of string literals.

  • Back in the days of coding windows applications in c, there were similar files #included in windows which consisted of long lists of #defines creating constants. Various c applications emulated this approach in their own files. The "class" seems to be a "transliteration" of this "c-ism". The fundamental principle of Object Oriented Design is to mix code and data into related functional units: objects. As jfullerton wrote:

    From a programming point of view, object-orientation involves program objects, encapsulation, inheritance, and polymorphism. The conceptual objects are modeled in the program code. Encapsulation keeps an object's data and methods that use the data together as part of the object.

    So clearly, this constant list does not conform to OO practices but is a throw back to the old days.

    To answer your questions:

    1. -- This class holds constants, that is it
    2. -- The old developer probably did this because that was what he was used to doing
    3. -- It's not current best practices.

    Naturally, if this is part of your application, you can't just throw this away. Rather, this is something to refactor over time, assuming you use current best practices of Test Driven Development and Refactoring

    EnocNRoll : This code reminds of .h files too.
    Scott Dorman : There are still very valid reasons for using classes consisting of constants. Not everything in the world of programming can/will conform to pure OO ideals. See some of the other responses for more explanations. This isn't neccessarily something that should be refactored away.
    StingyJack : This kind of list is nice when you work with a alot of datasets/datatables and need to reference columns or tables from many parts of an app.
    Refracted Paladin : @stingyJack- can you elaborate because that is what we have. How is it nice or is that a second post.. :)
  • It really looks like a naive implementation of a string table.

    The values are no magic strings and "easy" system-wide changes. I would argue that a resources file is easier to both implement and maintain.

    Is your colleague's implementation a best practice? I'd say no. Using string tables, it is, especially if you need internationalization in your app.

  • Putting string constants into a separate class is a best practice in many situations, however, this is a poor example. A better way would be to create a StringConstants namespace and then organize the strings so that related string constants are organized into separate classes. This is just a poor implementation of a good idea.

    If globalization is a concern, then (as others have pointed out), strings should be kept in resource files.

0 comments:

Post a Comment