3

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?

  • 8
    I think you may have some terminology problems. A username in most nomenclatures, refers to a string that is unique to an individual or a group. As such it is represented as a string of characters. A User would have an attribute of username as well as userId, fullName, etc. – BobDalgleish Aug 6 '14 at 16:41
  • 1
    @BobDalgleish Fair point. In retrospect this should be a User class with Username, etc. As you said. – Elias Aug 6 '14 at 16:42
  • 4
    In addition, a User, UserName or UserID would not look up itself. That assumes knowledge of the underlying data store, knowledge which User, UserName and UserID should not require. – Robert Harvey Aug 6 '14 at 16:42
  • 2
    He's saying it's wrong for the User class to be responsible for retrieving user data from the database (and he's right). This is unnecessary coupling. Users should be plain old values, preferably immutable. – Doval Aug 6 '14 at 17:48
  • 2
    @Elias Create a class/function that hits up Active Directory and creates instances of User based on the info it retrieves. This makes the Active Directory code depend on User 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 the User class, even though the code using User is interested in the data, not where it came from. – Doval Aug 6 '14 at 18:44
3

*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 from User - I know, that sucks... but you can create a new class and expose a User 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 current User 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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.