Replace a Module with a Class and Shared Member Question

[BukHix]

I want to replace the Public Declarations that I currently store in a Module with a Class.

Currently in my Module I have a declaration like this:

Public SetPath As String = My.Computer.FileSystem.SpecialDirectories.MyDocuments

How do I get the same functionality from a Class?

Public Class Audits

    Private _SetPath As String
    Public Property SetPath() As String
        Get
            Return _SetPath
        End Get
        Set(ByVal value As String)
            _SetPath = value
        End Set
    End Property

End Class

I know I can reference the SetPath in code by doing a Dim aud as New Audit so my question is this. Where is the correct place to put the

= My.Computer.FileSystem.SpecialDirectories.MyDocuments

And is this an good example of a member that should be shared? If so does both the Private and the Public need to be shared. I just started using Refactor Pro and it almost always is recommending that I make all members shared. Is there ever a good reason not to do so?

[Shaggy Hiker]

If you DO decide to share it, then you have just wasted your time by moving from a module, because a module is nothing but a class with all shared members. You don't reference it quite the way you would if you were to actually make a class with all shared members, but only because MS decided to make modules look and act as they did in VB6, so the conversion into a class is done quietly in the background, and you aren't required to use the module name as you would if this was a class. Again, that's only because MS wanted modules to act in a familiar way for VB6 people, not because they actually are different animals.

There are MANY good reasons not to make members shared. Is Refactor Pro recommending that most members of most classes be shared, or just members when you are converting a module to an explicit class?

If you have a shared member, then there is only one instance of that member for all instances of the class. Change it in one instance, you change it for all. If that's the way it SHOULD work, then the member should be shared, otherwise it must not be.

[NickThissen]

I'm not sure about Refactor Pro, but ReSharper (which is probably a very similar tool) always tells me when a method can be declared shared. It doesn't suggest to make it shared, it simply notifies me that it can be made shared. This isn't much help if you are in the early design stages, but if you have most of your design ready and Refactor is telling you that a method can be declared shared, you might notice that that would indeed by a good change. I'm pretty sure (at least in the case of ReSharper) this suggestion is only made when there can be no disadvantage to using a shared method. Of course, the final decision is up to you and you should always make sure that it does not break any functionality.


In general I find this a good rule of thumb:
If a method or property is specific to an instance of your class, then don't make it shared. If the method or property is not specific to any instance, but merely specific to that class, then make it shared.

Example: the Point structure has properties like the X and Y values, which are obviously specific to a single instance of the Point class (one point), so they are not shared. There is also a property 'Empty' though, that returns a new instance with X and Y set to 0. That property is not specific to any instance, so it is shared.


EDIT
As to your actual question, if you want to retain the same functionality that the module provided, then the property should be shared (because everything in a module is shared). Because the property accesses the private field, the private field must also be shared (the IDE will tell you this automatically). In that case, I think you can set the initial value in the declaration of the private field, eg

Private Shared _SetPath As String = My.Computer.FileSystem.SpecialDirectories.MyDocuments

But, as Shaggy Hiker suggested, you may as well use a module because it will result in the same behavior. It doesn't really matter though, and in many cases I like to use a class better, because it then forces you to specify the name of the class before using the method, which makes it easier to read. Whenever I see a method/property call like this

SetPath = "Abc"

I automatically assume that I can find 'SetPath' in the current class. If SetPath is in a module however that isn't the case, but the code is still valid. If you make SetPath a shared method in a class Audits, then you must use

Audits.SetPath = "Abc"

which immediately tells me that the property is in fact in the class Audits. Problem solved.


Finally, by the way, I think 'SetPath' is not a very good name for a property, because you can also 'get' it. Perhaps just 'Path' is a better name? Properties usually do not specify 'Get' or 'Set' in their name (while functions often do).

[BukHix]

Well there have been a few times that it didn't recomemend making it shared but it most instances it does. At least in my experience but most of my applications are very small, single function, in house apps so that could be a factor.

For some back ground....

I recently finished the book (2nd time through) Doing Objects with Visual Basic 2005 while I was also trying out the CodeRush/Refactor Pro tools.

The experience has been a "religious" one for me and I am definitely on the path to writing more clear, maintainable code. I am really excited about OOP and have been refactoring all my current code to be more OOPish. There is still a lot that doesn't make sense to me and in many cases I am sure that I am just transferring one set of bad habits into another set of bad habits (for instance, potentially, moving all my public declarations to shared members), which is why I asked the question. The Book and Refactor Pro have been excellent in helping me get to a point but I still lack a lot of the knowledge necessary to fill in all the gaps between the event driven method and the Object Orientated method.

Basicaly I don't want to Refactor because I can. I want to do it becuase it is the correct thing to do. Figuring out the difference is the hard part and I am sure one that I am going to struggle with for a long time to come.

[techgnome]

Basicaly I don't want to Refactor because I can. I want to do it becuase it is the correct thing to do.

Therein lies the problem... there isn't a hard and fast rule... only general guidelines and gut instinct. A lot depends on the situation. The rule of thumb Nick pointed out is generally how most of us deal with it... but even with that, there are times when to break that thumb and do it another way.

-tg

[BukHix]

NickThissen and Shaggy Hiker thank you for your answers. NickThissen thanks for the update. That really helped clear it up for me.

[Shaggy Hiker]

I'd have to say that you are on the right track in general. When something should or shouldn't be shared is not always clear, and is often kind of irrelevant. In your initial example, you have a string that is shared (since it is in a module). Whether you leave it shared in a class can be thought of in these ways:

1) From the perspective of performance and size: It's largely irrelevant. The size of adding an additional, short, string, can be ignored. The time taken to create it is also insignificant, though there is slightly more time involved. On the other hand, if the object in question was something costly, such as a database connection, then other factors would come into play.

2) From the perspective of versatility of the class: If you make that particular string shared, since it holds a directory path, then it can only ever hold that one path. If you ever want an instance of the class that holds a different path...well tough, you made it shared, there will only ever be the one instance.

Overall, though, you are thinking about it, and that is half the battle.