[RESOLVED] Analog Clock with Roman Numbers as Text not displaying correctly. Just displays XII..

[nkvb]

Thread Sub : Analog Clock with Roman Numbers as Text not displaying correctly. Just displays XII throughout

Hello
I am trying to increase the value of RomanNumber from Array to get the display right. Somehow I am not getting it
Fail to understand why RomanNumber as Text in array not being increased
It just Accepting XII for all the twelve digits. How can I get rid of this and get all the 12 Roman Numbers

Will appreciate your help. Coded as Below

Private newImgClock As New Bitmap(700, 700)
Private gr As Graphics = Graphics.FromImage(newImgClock)
Private cPoint As Point = New Point(newImgClock.Width / 2, newImgClock.Width / 2)
Private imgWidth As Integer

Private Sub BtnViewClock_Click(sender As Object, e As EventArgs) Handles BtnViewClock.Click

        Dim cPoint As Point = New Point(newImgClock.Width / 2, newImgClock.Width / 2)
        Dim radiuss As Integer = newImgClock.Width / 2
        NewDrawRomanClockFace(gr, cPoint, radiuss)

End Sub

Public Sub NewDrawRomanClockFace(ByVal g As Graphics, ByVal centre As PointF, ByVal radius As Single)

        Dim RomansArray As String() = {"XII", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX", "X", "XI"}

        For i As Integer = 0 To 59
            Dim angle As Double = Math.PI * 2 * i / 60 ' Calculate angle for each tick

            Dim outerX As Single = centre.X + CSng(Math.Cos(angle) * radius)
            Dim outerY As Single = centre.Y + CSng(Math.Sin(angle) * radius)


            Dim innerRadius As Single = If(i Mod 5 = 0, radius - (radius * 0.1F), radius - (radius * 0.05F))

            Dim innerX As Single = centre.X + CSng(Math.Cos(angle) * innerRadius)
            Dim innerY As Single = centre.Y + CSng(Math.Sin(angle) * innerRadius)

            Dim innerRadiusForFont As Single = radius - (radius * 0.1F) - 50

            Dim innerXfont As Single = centre.X + CSng(Math.Cos(angle) * innerRadiusForFont)
            Dim innerYfont As Single = centre.Y + CSng(Math.Sin(angle) * innerRadiusForFont)

            'Dim txt As String
            Dim sizeFont As Integer = 72 

            If (i Mod 5 = 0) Then
                g.DrawLine(New Pen(Color.DarkGreen, 3), New PointF(outerX, outerY), New PointF(innerX, innerY))

                Dim h As Integer = (i \ 5 + 3) Mod 12
                If h = 0 Then h = 12
                'txt = h.ToString()

                For j = 0 To UBound(RomansArray)
                    RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (i + 5) / 60), innerXfont, innerYfont)
                    j = (j + (CSng(360 * (j + 5) / 60)))
                Next j
            Else
                g.DrawLine(New Pen(Color.Orange, 2), New PointF(outerX, outerY), New PointF(innerX, innerY))
            End If

        Next
        gr.DrawEllipse(New Pen(Color.Black, 2), New Rectangle(centre.X - radius, centre.Y - radius, radius * 2, radius * 2))
        PictureBox1.Image = newImgClock

    End Sub

Private Sub RomanNosRotatedText(ByVal gr As Graphics, ByVal txt As String, ByVal sizeFont As Integer, ByVal angle As Single, ByVal cx As Double, ByVal cy As Double)

        Dim string_format As New StringFormat
        string_format.Alignment = StringAlignment.Center
        string_format.LineAlignment = StringAlignment.Center

        Dim graphics_path As New GraphicsPath(Drawing.Drawing2D.FillMode.Winding)
        Dim ix As Integer = CInt(cx)
        Dim iy As Integer = CInt(cy)

        Dim nFont As New Font("Arial", 12, FontStyle.Bold)
        graphics_path.AddString(txt, nFont.FontFamily, FontStyle.Bold, sizeFont, New Point(ix, iy), string_format)

        Dim rotation_matrix As New Matrix

        graphics_path.Transform(rotation_matrix)
        gr.FillPath(Brushes.DarkRed, graphics_path)

    End Sub

Thanks nkvb

[jmcilhinney]

Have you debugged your code? Have you set a breakpoint and stepped through it line by line to see with your own eyes that it is doing what you expect it to do? As a developer, you don't just look at the visual result of your code. You look at the state of the code as it executes. I suspect that the issue will be quite obvious if you do that. Before doing that though, you might stop to think what the actual logic is that you're trying to implement and whether that code actually implements it. Specifically, why do you think that those nested For loops are a good idea? Do you really want to loop through the entire RomansArray every time (i Mod 5 = 0) is True? I would think not.

[Poppa Mintin]

Hi nkvb,
Surly the best way to get your clock face absolutely as you want it would be to draw it in Form1.vb [Design].
Clearly you already have the main basis with the image you've used in this thread.

Poppa

[nkvb]

Dear jmcSir, Thank you for your suggestion.

Have you debugged your code? Have you set a breakpoint and stepped through it line by line to see with your own eyes that it is doing what you expect it to do? As a developer, you don't just look at the visual result of your code. You look at the state of the code as it executes. I suspect that the issue will be quite obvious if you do that. Before doing that though, you might stop to think what the actual logic is that you're trying to implement and whether that code actually implements it. Specifically, why do you think that those nested For loops are a good idea? Do you really want to loop through the entire RomansArray every time (i Mod 5 = 0) is True? I would think not.

I tried to debug but was not successful, Even it showed me Index out of bounds with your suggestion.

Although very clear in your comment but If few more hints or comments you could provide. I could still put efforts and work on the same

Dear FordPrefect

First of all. It seems to me that all you have done is copy some code from the Internet, added a few bits and pieces of your own badly made code snippets and hoped everything would magically gel together.

Quite Obvious that is with Option Strict Off

This is the same code with your Reply in #2 in thread 909744 and in that thread you did not demand as below. Please suggest

Option Strict On
Option Explicit On
Option Infer Off

Only I changed the subroutine references name you had added DoClock() and calling the Doclock statements and arguments in BtnViewClock_Click
in the above mentioned thread
where as this time i did not create DoClock() and just called the subroutines and added its respective references
and on basis of the code you provided I worked on to get the desired result and therefore i tried to incroporate the nested For Next Loops
and did not succeed.

Dear Poppa Mintin

Surly the best way to get your clock face absolutely as you want it would be to draw it in Form1.vb [Design].
Clearly you already have the main basis with the image you've used in this thread.

I will try not to draw anything on Forms if anything to draw will be in picturebox always. But still it depends if required
and not even use the Form'sPaint_event nor PictureBox Paint only if required

[nkvb]

FordPrefect

Hi

Yes, I had those Options in previous post. There is a reason for that. You really need to include those 3 lines at the very top of *any* code you write/post. There are so many errors in the code you posted that could be avoided by fixing errors caught by having those Options.

You declare a Matrix but don't actually use it. e.g. rotation_matrix.RotateAt(angle, New PointF(cx, cy))
(just an example)

If you submit a large block of code which will not compile due to a huge number of errors that you should be able to fix yourself then many people will just ignore your post.

On basis of the Last code only I worked further
As suggested Let me work on it and provide new code. BTW will remove rotation_matrix.
and till today I've not worked with below mentioned FYI

Option Strict On
Option Explicit On
Option Infer Off

[jmcilhinney]

I tried to debug but was not successful

I don't know what that actually means. If you don't know how to debug code, learn that first. If you do know, you should have stepped through the code line by line and seen exactly what the code was doing that didn't make any sense. Again, put some thought into the logic first, then only write code to implement that logic. There's no way that the code you have actually implements logic that makes any sense. Can you actually explain your logic?

Even it showed me Index out of bounds with your suggestion.

Don't be so vague. Presumably the code you already have runs without error, despite producing the wrong results. If you changed the code and now an exception is thrown, you need to actually show us what changes you made and specify the exact error message and where it occurs if you expect us to be able to help you.

[nkvb]

Jmc Sir

If you don't know how to debug code, learn that first

I don't know
I did try by clicking on the Line No where syntax For j = 0 To UBound(RomansArray) Starts and pressed F9
and tried to check the results for i, h and j it gave following results by pressing Continue Button 12 times

i =  0,  5,  10,  15,  20, 25, 30, 35, 40, 45, 50, 55
h =  3,  4,   5,   6,   7,  8,  9, 10, 11, 12,  1,  2 
j =  0, 31,  31,  31,  31, 31, 31, 31, 31, 31, 31, 31

Hope I on correct track

Don't be so vague. Presumably the code you already have runs without error, despite producing the wrong results. If you changed the code and now an exception is thrown, you need to actually show us what changes you made and specify the exact error message and where it occurs if you expect us to be able to help you

After FordPrefect's comments I deleted the part where error was thrown. I am afraid sir, Now I don't remember.

[nkvb]

Jmc Sir,

I think in the following manner i worked and error was thrown

Before the syntax If (i Mod 5 = 0)
Dim j As Integer
RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (j + 5) / 60), innerXfont, innerYfont)
j = (j + (CSng(360 * (j + 5) / 60)))

Revised Code Where Error was thrown marked in Red

Imports System.Drawing
Imports System.Drawing.Drawing2D

Public Class RomanAnalogClock

    Private newImgClock As New Bitmap(700, 700)
    Private gr As Graphics = Graphics.FromImage(newImgClock)
    Private cPoint As Point = New Point(Convert.ToInt32(newImgClock.Width / 2), Convert.ToInt32(newImgClock.Width / 2))
    Private imgWidth As Integer

    Private Sub BtnViewClock_Click(sender As Object, e As EventArgs) Handles BtnViewClock.Click

        Dim cPoint As Point = New Point(newImgClock.Width / 2, newImgClock.Width / 2)
        Dim radiuss As Integer = newImgClock.Width / 2
        NewDrawRomanClockFace(gr, cPoint, radiuss)

    End Sub

    Public Sub NewDrawRomanClockFace(ByVal g As Graphics, ByVal centre As PointF, ByVal radius As Single)

        Dim RomansArray As String() = {"XII", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX", "X", "XI"}

        For i As Integer = 0 To 59
            Dim angle As Double = Math.PI * 2 * i / 60 ' Calculate angle for each tick

            Dim outerX As Single = centre.X + CSng(Math.Cos(angle) * radius)
            Dim outerY As Single = centre.Y + CSng(Math.Sin(angle) * radius)


            Dim innerRadius As Single = If(i Mod 5 = 0, radius - (radius * 0.1F), radius - (radius * 0.05F))

            Dim innerX As Single = centre.X + CSng(Math.Cos(angle) * innerRadius)
            Dim innerY As Single = centre.Y + CSng(Math.Sin(angle) * innerRadius)

            Dim innerRadiusForFont As Single = radius - (radius * 0.1F) - 50

            Dim innerXfont As Single = centre.X + CSng(Math.Cos(angle) * innerRadiusForFont)
            Dim innerYfont As Single = centre.Y + CSng(Math.Sin(angle) * innerRadiusForFont)

            'Dim txt As String
            Dim sizeFont As Integer = 72

            Dim j As Integer
            RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (j + 5) / 60), innerXfont, innerYfont)
            j = (j + (CSng(360 * (j + 5) / 60)))
            

            If (i Mod 5 = 0) Then
                g.DrawLine(New Pen(Color.DarkGreen, 3), New PointF(outerX, outerY), New PointF(innerX, innerY))

                Dim h As Integer = (i \ 5 + 3) Mod 12
                If h = 0 Then h = 12
                'txt = h.ToString()

                'For j = 0 To UBound(RomansArray)
                'RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (i + 5) / 60), innerXfont, innerYfont)
                'j = (j + (CSng(360 * (j + 5) / 60)))
                'Next j
            Else
                g.DrawLine(New Pen(Color.Orange, 2), New PointF(outerX, outerY), New PointF(innerX, innerY))
            End If

        Next

        gr.DrawEllipse(New Pen(Color.Black, 2), New Rectangle(centre.X - radius, centre.Y - radius, radius * 2, radius * 2))
        PictureBox1.Image = newImgClock

    End Sub

    Private Sub RomanNosRotatedText(ByVal gr As Graphics, ByVal txt As String, ByVal sizeFont As Integer, ByVal angle As Single, ByVal cx As Double, ByVal cy As Double)

        Dim string_format As New StringFormat
        string_format.Alignment = StringAlignment.Center
        string_format.LineAlignment = StringAlignment.Center

        Dim graphics_path As New GraphicsPath(Drawing.Drawing2D.FillMode.Winding)
        Dim ix As Integer = CInt(cx)
        Dim iy As Integer = CInt(cy)

        Dim nFont As New Font("Arial", 12, FontStyle.Bold)
        graphics_path.AddString(txt, nFont.FontFamily, FontStyle.Bold, sizeFont, New Point(ix, iy), string_format)

        Dim rotation_matrix As New Matrix

        graphics_path.Transform(rotation_matrix)
        gr.FillPath(Brushes.DarkRed, graphics_path)

    End Sub
End Class

[Shaggy Hiker]

You don't know how to debug code? There are a couple simple steps. The first is to add a breakpoint, which you can do a variety of ways, the easiest of which is to click in the left margin of the code page. If you click in the right place, a maroon ball will appear there and the line will be highlighted in maroon.

This would probably be a good line to put the breakpoint on:

For i As Integer = 0 To 59

though it might also be on the next line inside the loop, or on the line where you call the method. There are advantages to each.

You then run the code. Execution will stop on the breakpoint, at which time you can look at the contents of any variable that is in scope, either by hovering the mouse over it or by clicking on it and pressing Shift+F9. You can then step forwards line by line using either F10 or F11. I'll leave it up to you to determine what the difference between the two are, and either will work fine to start with.

By combining those steps (the breakpoint, examining the contents of variables, and stepping through the code) you will be able to see what each line of code is doing and be able to figure out why it is doing it. With those steps you can understand the code, without those steps you are just guessing.

[OptionBase1]

You need to spend some time reading about the concepts of variable declaration and scope. I can't think of any obvious cases where it is correct to repeatedly Dim a variable inside a loop, but perhaps there are some. In your case, it is not correct and is contributing to some of the issues that you are encountering.

In your new code above, in the second line you've marked as red, j will always have a value of 0 for every iteration through your loop because you are declaring j as a new variable in the line immediately above it, which will assign it a default value of 0.

[Shaggy Hiker]

I'd also say that I don't feel as strongly about having those Options at the top of each file. I think I understand why Ford wants them visible, as I expect that he doubts you have set them all at the project scope. You SHOULD have them set at the project scope. By default, Option Explicit will be ON, and coding without that being on is insane. Option Strict is OFF by default, and it should be turned ON by default. That will prevent a lot of bugs...and inefficient code, but mostly bugs. The errors that result from turning that ON are real errors, even if the code ran with Option Strict OFF.

I don't feel all that strongly about Option Infer. I prefer to have it ON, but when you are starting out, I can understand wanting to have it OFF, as it forces you to understand what you are doing better.

[Shaggy Hiker]

Yeah, I figured that was what you are getting at. The lines shouldn't be posted in every file, they should be set as the defaults for Visual Studio itself (in Tools | Options) or for the project, but if those things aren't done, then they DO need to be added to the files.

[.paul.]

Talking about Options, I rarely take advantage of Option Infer. I know jmcilhinney is a big fan of it. What does everyone else think of it?

[.paul.]

Hi

Don't like it. I prefer to set the type manually, and, I prefer to see the type in the listing.

[nkvb]

SH Sir,

Thank you for the informative steps for debugging the code. from For i As Integer = 0 to 59
i, h,j value remains the same as per my reply #9 and RomansArray(j) = XII throughout

and also remains same after changing to revised code below

FordPrefect

Please find the revised code as per your suggestion
The same result displaying i.e XII for all the hours

Option Strict On
Option Explicit On
Option Infer Off

Imports System.Drawing
Imports System.Drawing.Drawing2D

Public Class RomanAnalogClock

    Private newImgClock As New Bitmap(700, 700)
    Private gr As Graphics = Graphics.FromImage(newImgClock)
    Private cPoint As Point = New Point(Convert.ToInt32(newImgClock.Width / 2), Convert.ToInt32(newImgClock.Width / 2))
    Private imgWidth As Integer

    Private Sub BtnViewClock_Click(sender As Object, e As EventArgs) Handles BtnViewClock.Click

        Dim radiuss As Integer = Convert.ToInt32(newImgClock.Width / 2)


        NewDrawRomanClockFace2(gr, cPoint, radiuss)

    End Sub

    Public Sub NewDrawRomanClockFace2(ByVal g As Graphics, ByVal centre As PointF, ByVal radius As Single)

        Dim RomansArray As String() = {"XII", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX", "X", "XI"}

        For i As Integer = 0 To 59
            Dim angle As Double = Math.PI * 2 * i / 60 ' Calculate angle for each tick

            Dim outerX As Single = centre.X + CSng(Math.Cos(angle) * radius)
            Dim outerY As Single = centre.Y + CSng(Math.Sin(angle) * radius)


            Dim innerRadius As Single = If(i Mod 5 = 0, radius - (radius * 0.1F), radius - (radius * 0.05F))

            Dim innerX As Single = centre.X + CSng(Math.Cos(angle) * innerRadius)
            Dim innerY As Single = centre.Y + CSng(Math.Sin(angle) * innerRadius)

            Dim innerRadiusForFont As Single = radius - (radius * 0.1F) - 50

            Dim innerXfont As Single = centre.X + CSng(Math.Cos(angle) * innerRadiusForFont)
            Dim innerYfont As Single = centre.Y + CSng(Math.Sin(angle) * innerRadiusForFont)

            'Dim txt As String
            Dim sizeFont As Integer = 72
            Dim j As Integer
     
            If (i Mod 5 = 0) Then
                g.DrawLine(New Pen(Color.DarkGreen, 3), New PointF(outerX, outerY), New PointF(innerX, innerY))

                Dim h As Integer = (i \ 5 + 3) Mod 12
                If h = 0 Then h = 12
                'txt = h.ToString()
                For j = 0 To UBound(RomansArray)
                    RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (i + 5) / 60), innerXfont, innerYfont)
                    j = Convert.ToInt32(j + (CSng(360 * (j + 5) / 60)))
                Next j


            Else
                g.DrawLine(New Pen(Color.Orange, 2), New PointF(outerX, outerY), New PointF(innerX, innerY))
            End If

        Next

        Dim locRectX As Integer = Convert.ToInt32(centre.X - radius)
        Dim locRectY As Integer = Convert.ToInt32(centre.Y - radius)
        Dim newRadius As Integer = Convert.ToInt32(radius)

        gr.DrawEllipse(New Pen(Color.Black, 2), New Rectangle(locRectX, locRectY, newRadius * 2, newRadius * 2))

        PictureBox1.Image = newImgClock

    End Sub

    Private Sub RomanNosRotatedText(ByVal gr As Graphics, ByVal txt As String, ByVal sizeFont As Integer, ByVal angle As Single, ByVal cx As Double, ByVal cy As Double)

        Dim string_format As New StringFormat
        string_format.Alignment = StringAlignment.Center
        string_format.LineAlignment = StringAlignment.Center

        Dim graphics_path As New GraphicsPath(Drawing.Drawing2D.FillMode.Winding)
        Dim ix As Integer = CInt(cx)
        Dim iy As Integer = CInt(cy)

        Dim nFont As New Font("Arial", 12, FontStyle.Bold)
        graphics_path.AddString(txt, nFont.FontFamily, FontStyle.Bold, sizeFont, New Point(ix, iy), string_format)

        Dim rotation_matrix As New Matrix

        gr.FillPath(Brushes.DarkRed, graphics_path)

    End Sub

End Class

Thanks everyone for your views for three lines. Henceforth will keep in mind and code accordingly

[OptionBase1]

In your new code you are still drawing all of the roman numeral text on the clock face 12 times in total. There's no reason for the loop that draws the roman numeral text to be nested inside of your i loop.

Also, it isn't at all clear why you are using a global graphics object and then passing that object to a sub but then ignoring the parameter and using the global variable instead. Stuff like that makes your code very, very difficult to follow.

[jmcilhinney]

In your new code you are still drawing all of the roman numeral text on the clock face 12 times in total. There's no reason for the loop that draws the roman numeral text to be nested inside of your i loop.

Also, it isn't at all clear why you are using a global graphics object and then passing that object to a sub but then ignoring the parameter and using the global variable instead. Stuff like that makes your code very, very difficult to follow.

This is why I'm trying to get the OP to spend some time thinking about the actual logic first, but they seem resistant to doing so. Not unusual, as writing code is the fun part and that's what everyone wants to do, but then we end up in situations like this where they can't understand why the code doesn't do what they want when they can't even really explain what they want the code to do, other than the final result. If you can't explain the desired logic step by step, how could you possibly write code to implement it?

[Shaggy Hiker]

Consider this part of the code:

               For j = 0 To UBound(RomansArray)
                    RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (i + 5) / 60), innerXfont, innerYfont)
                    j = Convert.ToInt32(j + (CSng(360 * (j + 5) / 60)))
                Next j

The variable j starts at 0, then each time through the loop it would increment by 1 until it reached the upper bound of the array. However, inside the loop, you are setting j to some other value. So, the loop would have j start at 0, then go to 1, then 2, 3, 4, etc. But you start j at 0, then go to (0 + 360 * 5/60), which would end up being 30. If that were to increment, then the second time through the loop, j would be 31, which would be out of the range of the array, except that it's also way above the upper end of the loop.

Frankly, I'm not quite sure what that would do, as I don't think I've ever tried something like that. Whatever it is doing, it isn't right.

[nkvb]

OptionBase1

In your new code you are still drawing all of the roman numeral text on the clock face 12 times in total. There's no reason for the loop that draws the roman numeral text to be nested inside of your i loop.

FYI Retained everything same as per OP.

There's no reason for the loop that draws the roman numeral text to be nested inside of your i loop.

How would one get the value of Roman Numeral if not nested in a loop. This is what I've learnt and implemented so far.

Also, it isn't at all clear why you are using a global graphics object and then passing that object to a sub but then ignoring the parameter and using the global variable instead. Stuff like that makes your code very, very difficult to follow.

Hope you are commenting on

Private gr As Graphics = Graphics.FromImage(newImgClock)

the reason was to share globally was to for eg If I wanted to draw another circle just below the hours inside the clock
So only thing I need incorporate the PictureBox1.Image = newImgClock in respective sub routines where ever required
and did not have to Dim gr As Graphics = Graphics.FromImage(newImgClock) in each sub rouitne
Other Globally shared variables
Private cPoint As Point = New Point(newImgClock.Width / 2, newImgClock.Width / 2)
this was to share centre of Circle Globally.

like wise ...... New Rectangle(centre.X - radius, centre.Y - radius, radius * 2, radius * 2)) will also get the common Centre.X andCentre.Y and Radius. this was the logic.

[nkvb]

Yes Got it Right and implemented in both the codes

.............
If (i Mod 5 = 0) Then
                g.DrawLine(New Pen(Color.DarkGreen, 3), New PointF(outerX, outerY), New PointF(innerX, innerY))

                Dim h As Integer = (i \ 5 + 3) Mod 12
                If h = 0 Then h = 12
                'txt = h.ToString()

                Dim j As Integer
                j = (i \ 5 + 3) Mod 12

                'txt = h.ToString()
                RomanNosRotatedText(gr, RomansArray(j), sizeFont, CSng(360 * (i + 5) / 60), innerXfont, innerYfont)
              
            Else
                g.DrawLine(New Pen(Color.Orange, 2), New PointF(outerX, outerY), New PointF(innerX, innerY))
            End If
..........

NOTE: have removed j = j + 1 Not required at all
Will await FordPrefect's new logic

[nkvb]

FordPrefect

What ever happened to the '2 colour segmented' code?

I did not get you.

Are you referring to thread 909832

[nkvb]

Hi

Yes, that is the one. Did you use it somewhere?

Very Much.

Thanks
nkvb