0
\$\begingroup\$

In my project, I have three different userforms to display, optimized for three different aspect ratios. I need my code to call the screen resolution of the user's monitor, then check to see which category it belongs to, and finally choose the appropriate userform.

I've gotten as far as categorizing, but I'm self-taught and have been using absolutely horrendous code. I've just taken every aspect ratio available on our machines and put it into an if then statement; when the correct if statement is triggered, it populates the variable Asp.

I'm absolutely certain there's a better way to do this. I would like to;

  1. find a mathematical way to represent a "range" of ratios, as not every resolution in the 16:9 category, for example, is exactly 16/9, and
  2. clean up the validation system that picks the right category for the user's setup. Possibly an array or a Case statement?

Declare Function getsystemmetrics32 Lib "User32" _
    Alias "GetSystemMetrics" (ByVal nIndex As Long) As Long

Sub callAspectRatio()

Dim w As Long
Dim h As Long
Dim Asp As String

w = getsystemmetrics32(0)
h = getsystemmetrics32(1)

If w = 1920 And h = 1080 Then Asp = "16:9"
If w = 1680 And h = 1050 Then Asp = "16:9"
If w = 1600 And h = 900 Then Asp = "16:9"
If w = 1366 And h = 768 Then Asp = "16:9"
If w = 1360 And h = 768 Then Asp = "16:9"
If w = 1280 And h = 720 Then Asp = "16:9"
If w = 1280 And h = 600 Then Asp = "16:9"

If w = 1440 And h = 900 Then Asp = "8:5"
If w = 1280 And h = 1024 Then Asp = "8:5"
If w = 1280 And h = 960 Then Asp = "8:5"

If w = 1600 And h = 1200 Then Asp = "4:3"
If w = 1400 And h = 1050 Then Asp = "4:3"
If w = 1280 And h = 1024 Then Asp = "4:3"
If w = 1280 And h = 960 Then Asp = "4:3"
If w = 1152 And h = 864 Then Asp = "4:3"
If w = 1024 And h = 768 Then Asp = "4:3"
If w = 800 And h = 600 Then Asp = "4:3"

MsgBox Asp

End Sub
\$\endgroup\$
1
\$\begingroup\$

The firs thing worth pointing out is that you are ignoring a lot of special meaning with your current approach. Aspect ratios are a function determined by Width : Height and thus we can use an aspect ratio and an inputted Width and Height to get the aspect we want, instead of looking through a list that are just saying the same thing in different ways.

I took your original code and calculated each aspect ratio in order. Here are the results:

'    1.77777777777778 16:9
'    1.6 16:9
'    1.77777777777778 16:9
'    1.77864583333333 16:9
'    1.77083333333333 16:9
'    1.77777777777778 16:9
'    2.13333333333333 16:9
'    1.6 8:5
'    1.25 8:5
'    1.33333333333333 8:5
'    1.33333333333333 4:3
'    1.33333333333333 4:3
'    1.25 4:3
'    1.33333333333333 4:3
'    1.33333333333333 4:3
'    1.33333333333333 4:3
'    1.33333333333333 4:3

A reduced list would look like this:

'1.77... ' 16:9
'1.6     ' 16:9
'1.778   ' 16:9
'2.133   ' 16:9
'1.6     ' 8:5
'1.25    ' 8:5
'1.33... ' 8:5
'1.33... ' 4:3
'1.25    ' 4:3

Aside from the couple of instances of a single ratio being matched to two different aspects, they are fairly consistent. So then I checked each of the original aspects and their results:

?16/9
 1.77777777777778 
?8/5
 1.6 
?4/3
 1.33333333333333

In turn, we have a very simple function to get an aspect ratio from a width and height:

Public Function GetAspectRatio(ByVal Width As Long, ByVal Height As Long) As String
    Select Case Round(Width/ Height, 3)
    Case Is <= 1.33
        GetAspectRatio = "4:3"
    Case Is <= 1.6
        GetAspectRatio = "8:5"
    Case Is > 1.6
        GetAspectRatio = "16:9"
    Case Else
        ' Error : Ratio not pre-defined.
    End Select
End Function

Then your original code becomes this:

Declare Function GetSystemMetrics32 Lib "User32" _
    Alias "GetSystemMetrics" (ByVal nIndex As Long) As Long

Sub DisplayAspectRatio()
    Dim Width As Long
    Width = GetSystemMetrics32(0)

    Dim Height As Long
    Height = GetSystemMetrics32(1)

    MsgBox "Your aspect ratio is : " & GetAspectRatio(Width, Height)
End Sub

We could improve this a bit further by employing the use of constant values and such, but overall this will point you in the right direction.

Note that, with the improved code, I made sure to use clear variable names. Most users will understand what h means in context, but Height is clear to any user. Even better would be WindowHeight so that, even if your reader was completely dumb to reading code, they would know what that variable was. Same goes for naming Functions, and Subroutines.

Best of luck!

\$\endgroup\$
0
\$\begingroup\$

Well I didn't test it with all of your values, but after a quick check this should work:

if (w/16) = (h/9) Then 
    Asp = "16:9"
Else If (w/8) = (h/5) Then
    Asp = "8:5"
Else If (w/4) = (h/3) Then
    Asp = "4:3"
End If  

You should be consistent with the coding style you use. Right now you have variables which start with lowercase letter (w and h) and uppercase letter (Asp). You will do yourself a favour if you choose one style and stick to it.
If you decide to move on with programming like learning C# or VB.NET you should use camelCase casing to name variables and method parameters and PascalCase casing to name methods and classes.

\$\endgroup\$
0
\$\begingroup\$

Brandon's answer worked in spectacular form! I didn't quite know how to use a case statement, but this was an excellent example. Thank you so much for taking the time to explain everything and give me pointers on good code etiquette. New draft looks like this:

Declare Function getsystemmetrics32 Lib "User32" _
    Alias "GetSystemMetrics" (ByVal nIndex As Long) As Long

Sub callAspectRatio()

Dim WindowLength As Long
Dim WindowHeight As Long
Dim MyAspectRatio As Single
Dim OptimizedAspect As String

'calls screen length & width and divides l/w to return aspect ratio
WindowLength = getsystemmetrics32(0)
WindowHeight = getsystemmetrics32(1)
MyAspectRatio = WindowLength / WindowHeight

'matches your aspect ratio to closest available
Select Case MyAspectRatio
    Case Is < 1.33333333
        OptimizedAspect = "4:3"
    Case Is > 1.77777778
        OptimizedAspect = "16:9"
    Case Else
        OptimizedAspect = "8:5"
End Select

MsgBox OptimizedAspect

End Sub
\$\endgroup\$
  • \$\begingroup\$ Not a problem, glad to help! Also, I would highly recommend keeping the GetAspectRatio part abstracted to a new function. You mentioned you're self-taught so it is worth noting that I did that very intentionally to separate the alert logic (messagebox) from the definition logic (getting the ratio), and this separation of responsibilities becomes very important as your code becomes more complex. Learning to use function calls now will help you tremendously later. \$\endgroup\$ – Brandon Barney Nov 30 '17 at 20:47
  • \$\begingroup\$ I'm not super familiar with functions - I'll make sure to do some snooping around youtube and learn a little more about those! Thanks again :) \$\endgroup\$ – Lindy Newman Dec 1 '17 at 21:06

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.