I am attempting to encapsulate several features about a user in a single class. Although the main use for this class would be to initialize it once and never have to call set any variables again, I would like this class to be scalable to be called many times. Although by putting this into a class I feel like I am either over-complicating it or breaking OOP standards.
At the moment I am currently using these current sorts of calls throughout VBA code:
username = GetUsername()
OR
IF userHasSecurity(GetUsername(), "WhatUserHasAccessTo") = true THEN
What I am hoping to do it replace it (respectively) with:
Set UserID = New Username
UserID.GetUserName
OR
IF UserID.HasModuleAccess("WhatUserHasAccessTo") = true THEN
In addition to this change, I have a function that returns Active Directory
information and also verify if usernames input exist in Active Directory
The Class Diagram is as such
Private UserName As String
Private FirstName As String
Private MiddleInitial As String
Private LastName As String
Private Sub class_Initialize()
Private Function findCurrentUsername() As String
Private Function doesUsernameExist(usernameToCheck As String) As Boolean
Public Property Get GetUserName() As String
Public Property Let setUsername(newUsername As String)
Public Property Get getFirstName() As String
Private Property Let setFirstName(newFirstName As String)
Public Property Get getLastName() As String
Private Property Let setLastName(newLastName As String)
Public Property Get getMiddleInitial() As String
Private Property Let setMiddleInitial(newMiddleInitial As String)
Private Function findNameDetails()
Public Property Get getFullName() As String
Public Function HasModuleAccess(moduleName As String, Optional appName As String) As Boolean
Public Function getUserActiveDirectoryGroups() As DAO.Recordset
Is this more an opinion based execution(standalone functions vs. class) thing or is there something to gain? Does this break OOP standards and am I using classes incorrectly?
1 Answer 1
*before sitting down to writing code etc. - search, research available resources, functions - basically don't reinvent the wheel...
There is an Environ() function in VBA that pretty much does what you are describing and trying to imitate by using your own wrapper class.. ask yourself is it worth it? do you really need it that much?
Rule of thumb
Stick with good encapsulation
you said:
Although the main use for this class would be to initialize it once and never have to call set any variables again,
Don't assume things that may break good encapsulation - don't assume you will never have to do X and Y with your class - there are different techniques to protect objects state through properties.
As already pointed out in the comment section - it's important that your class is rather named User
and not Username
.
It's a user
. If it's meant to be reusable meaning you can reuse it in many different projects - not have many instances of it then it shouldn't be depended on any other, unrelated, piece of your application - specially the active directory etc. If VBA supported parametarised constructors that could have been a different story based on some crucial circumstances but it does not so no argument is needed.
read this: Properties vs public variables in class modules
Hope you get the idea because as far as your User
class goes you don't really need some of the properties you currently declared as properties...
I mean, look - you are not storing User
's age -- you don't really need a Let
property to check whether the age passed is not negative or not greater than some huge number... You are only storing some strings like firstName
, lastName
, middleInitial
etc.. if you don't need to protect the access to change those then they should ideally be just public
variables - acting sort of like read/write properties - without the ability to verify the data being assigned to them - but does it matter? They're Strings
...
One good candidate for a property could be an Enumerable property UserAccessLevel
...
First you would probably want to define a public Enum like
Public Enum UserAccessLevel
ualReadOnly
ualWriteOnly
ualReadWrite
End Enum
then you could have a property like
private currentUserAccessLevel as UserAccessLevel
' cool stuff
Public Property Get ModuleAccess() as UserAccessLevel
ModuleAccess = currentUserAccessLevel
End Property
Public Property Let ModuleAccess(value as UserAccessLevel)
currentUserAccessLevel = value
End Property
That would be cool with all the intelli-sense tips for your enums etc...
Now, I have noticed this:
Public Function getUserActiveDirectoryGroups() As DAO.Recordset
And I have mixed feelings because
VBA doesn't support class polymorphism - so you can't derive a
CustomUser
fromUser
- I know, that sucks... but you can create a new class and expose aUser
as a property of the class etc. Too broad to discuss here though...getUserActiveDirectoryGroups()
may not necessarily be something that any other project may want to use. I mean it's cool to have it there as long as it's not dependant on any other external and unrelated to the currentUser
class members.
Say that the getUserActiveDirectoryGroups()
needs to know things that current User
class doesn't have access / doesn't know than it would probably be a better idea not to include that with the user class.
I could talk about this for another hour or two but I really hope at this point you feel a little bit more enlightened :)
Stick with good encapsulation.
from here
- Understand the object-oriented principle of Encapsulation.
- Learn the available modifiers for type members.
- Protect object state through properties.
- Control access to methods.
Explore related questions
See similar questions with these tags.
User
would have an attribute ofusername
as well asuserId
,fullName
, etc.User
class withUsername
, etc. As you said.User
class to be responsible for retrieving user data from the database (and he's right). This is unnecessary coupling.User
s should be plain old values, preferably immutable.User
based on the info it retrieves. This makes the Active Directory code depend onUser
but not vice-versa. The reason is wrong is that the coupling is unnecessary; every time there's a change in where the user data is stored you'll have to modify theUser
class, even though the code usingUser
is interested in the data, not where it came from.