[RESOLVED] vb.Net guidence from a highly professioned vb.Nut such as myself.

Now as to my knowledge no one else has done this here that I can find. I'm not quite sure where I should be posting this but I'm not going to search around and bother individual users to do so like I've noticed some do. So posting this thread here I feel is adequate.. Here it is... Plain and simple I want someone whom I can trust to just overlook my habits of coding with vb.net and to see if there is anything I can adjust before this project becomes way out of hand. I'm saying this because my current "sub" projects has just reached 2,000 lines and it's nowhere near complete. It contains a complex system of classes which are also new to me and that's were my main focus is at. This is the project I would like someone to look at and make comments wherever commenting is needed.

The project:
I'm working on quite an extensive project at the moment, well 2. One of which will actually be part of the other creating a whole project of many other projects. But since it's only me coding I feel one at a time is enough lol. The goal for this project is for it to be stand alone as well, which is why this one control contains nearly 20 classes. four of which are controls themselves. The sub project I'm asking for help on is my Schedule Grid Control, which will simply be like most business scheduling software.

Little Background:
My history is mostly with vb6 since I was 15, and being the way I was I pushed vb6 to it's limits. For instance I had my "winamp clone" media player which supported full skinning, anti-alias bordering with my image support being layers of png files. Using fMod, a third party supplier for audio processing mostly for gaming purposes which suited my needs perfect and I was able to accomplish a lot. Such as fading between songs, full bandwidth equalizer and a graphical equalizer as well! Not to mention it involved some multithreading as well which to my knowledge at the time was believed to be impossible with vb6.. which is wasn't :). Now having said all that the switch from 6 to .net for me was a tad bit rough, stopped coding for a few years then hoped into vb.net when 2010 was released. Bumpy at first, but I eventually began to flow. I began to love it! Just there are tons of small pieces of information I know I'm missing that most would learn through books or school.. For me I took classes in upper middle school but before that I had already taught myself enough to build a networking net send messengers which searched the schools network for every computer so as to select one and begin a chat.. If you could imagine this got me in trouble. But that was my only class room knowledge so EVERYTHING I HAVE LEARNED HAD BEEN THROUGH EXAMPLE. The college I'm at now has a horrible computer department and no classes for visual basic or any other language in fact. So I'm stuck till I transfer. This is why I signed up here :)

So if there is anyone here, preferably an MVP that could help I would greatly greatly! appreciate it.

if you post some of your code you've got more chance of getting a useful answer

the project is 2000+ lines... What I truly need looking at is it's class structure. which would be all 2000 lines of it.

My goal was to find someone whom I could send my project to and trust they will not attempt to stealing it. Which is why I'm only asking those who I clearly can tell have experience with visual basic and with helping those who need help. People who steal don't help others.

Alright with 52 views and no further than from where I started, I will attempt a butchered down code sample of what I'm attempting for my class structure...

vb Code:

Public Class AppointmentBookControl
    Inherits UserControl
 
    ' properties for which will be exposed to the user end.
 
    Private UserInterface As New AppointmentBookControlUserInterface(DirectCast(Me, AppointmentBookControl))
    Private Theme As New AppointmentBookControlTheme(DirectCast(Me, AppointmentBookControl))
 
 
    Protected Overrides Sub OnLoad(ByVal e As System.EventArgs)
        MyBase.OnLoad(e)
        serInterface.OnLoad(e)
        Me.Invalidate()
    End Sub
 
    Protected Overrides Sub OnPaint(ByVal e As System.Windows.Forms.PaintEventArgs)
        MyBase.OnPaint(e)
        Theme.OnPaint(e) ' Paint theme.
    End Sub
 
    Private Class AppointmentBookControlUserInterface
 
        <Browsable(False)> _
        Private Property Parent As AppointmentBookControl
 
        Private Toolbar As AppointmentBookControlToolbar
        Private DaySelectionbar As AppointmentBookControlDaySelectionBar
        Private MonthlyOverviewSlider As AppointmentBookControlMonthlyOverviewSlider
        Private ScheduleGridBody As AppointmentBookControlScheduleGridBody
 
 
        Sub New(ByRef Parent As AppointmentBookControl)
            MyBase.New()
            Me.Parent = Parent ' Pass on the reference.
            Toolbar = New AppointmentBookControlToolbar(Parent)
            DaySelectionbar = New AppointmentBookControlDaySelectionBar(Parent)
            MonthlyOverviewSlider = New AppointmentBookControlMonthlyOverviewSlider(Parent)
            ScheduleGridBody = New AppointmentBookControlScheduleGridBody(Parent)
        End Sub
 
        ' Nested classes for each of the four user interface divisions.
        Private Class AppointmentBookControlToolbar
 
            <Browsable(False)> _
            Private Property Parent() As AppointmentBookControl
 
            Private ReadOnly Property Skin() As AppointmentBookControlTheme.AppointmentBookControlThemeSkin
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Skin
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private ReadOnly Property [Region]() As Rectangle
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Structures.ToolBarRect
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private WithEvents Container As New AppointmentBookControl.AppointmentBookControlPanelControl
 
 
            Sub New(ByRef Parent As AppointmentBookControl)
                MyBase.New()
                Me.Parent = Parent ' Pass on the reference.
            End Sub
 
        End Class
 
        ' All other classes such as the one above fallow the same pattern.  I feel I don't need to list them.
        Private Class AppointmentBookControlDaySelectionBar
            ' Some code.
        End Class
 
        Private Class AppointmentBookControlMonthlyOverviewSlider
            ' Some code.
        End Class
 
        Private Class AppointmentBookControlScheduleGridBody
            ' Some code.
        End Class
 
    End Class
 
    Private Class AppointmentBookControlTheme
 
        <Browsable(False)> _
        Private Property Parent As AppointmentBookControl
 
        Public Structures As AppointmentBookControlThemeStructure
        Public Skin As New AppointmentBookControlThemeSkin
 
 
        Sub New(ByRef Parent As AppointmentBookControl)
            MyBase.New()
            Me.Parent = Parent ' Pass on the reference.
            Structures = New AppointmentBookControlThemeStructure(Parent)
        End Sub
 
        ' Nested Classes.
 
        Public Class AppointmentBookControlThemeStructure
            ' Contains structures which are all ReadOnly Properties so that when read will have an updated value which are calculated by the parent controls dimensions
        End Class
 
        Public Class AppointmentBookControlThemeSkin
            ' Contains ReadOnly properties containing colors, fonts, and etc.
        End Class
 
    End Class
 
End Class

Alright so that's the best I could do. There are numerous other classes within those and then some, but they all fallow the same idea.

Lines 6 & 7 - DirectCast(Me, AppointmentBookControl) ... totally unnecessary.... Me already IS an object typed as AppointmentBookControl... so that statement does nothing. Just pass Me.

Line 65... typically, at the class module, the general practice is to declare all your variables and stuff at the top.

Other than that, looks ok by me...

-tg

Quote:

---

Originally Posted by techgnome

Lines 6 & 7 - DirectCast(Me, AppointmentBookControl) ... totally unnecessary.... Me already IS an object typed as AppointmentBookControl... so that statement does nothing. Just pass Me.

Line 65... typically, at the class module, the general practice is to declare all your variables and stuff at the top.

Other than that, looks ok by me...

-tg

---

Thanks!

Also the second area you pointed out at Line 65.. That's actually a container such as a panel I have with some special styles applied to it when a new instance is initialized. Now each of my user interface classes contain one of these panels and that particular class handles that container and whatever controls may lie within them. My main class has nothing to do with those containers, just the classes the containers contain. It's almost my way of splitting up my main usercontrol into sections and then individually handling them by there own designated class.

Now picture this in 2000 lines.. you can see why I'm overwhelmed here. Sadly I can see this project surpassing triple what it is with ease.

Thanks again!

It doesn't matter what it is... generally speaking private members are declared at the top of their containing class/block.... in other words... before your properties of that containing class/block. so I would move line 65 up to line 42... that's all I'm saying.

typically the way I've seen it (and makes most sense to me) is the following order:

-- Constants
-- Event signatures & delegates
-- private classes
-- static members
-- public members (public variables)
-- private members (public variables)
-- static properties
-- private properties
-- friend properties
-- public properties
-- static functions and methods
-- private functions & methods
-- public functions & methods
-- Event handlers (which should be private)

Just a side note... not to discourage you or anything but 2000 lines for a project isn't much... try dealing with 200 PROJECTS in an app! -eek- THAT'S when things get confusing... that's also why it's importaint that you develop habits now in consistently structuring code the same way... this way when some one looks at any project you work on, whether it's proj #42 or #42000 ... they know they will find the variables at the top... or at the bottom, or what ever the shop standard is ...

-tg

Oh I see what you meant by all that now. I was miss understood thinking that you wanted me to place them at the most top, outside of the current class in which they are required to be in.. But no offense taken, this control itself is one of many Basically what I have done so far is it's "basic" elements.. Everything having to do with what you see, but still I have more than 90% still to finish before it's considered done and ready to be included within my master project which is going to be a Small to Large business management system for multiple business types(my goal) so before it wrote any more lines I figured I'd grab some advice before continuing. Because having gone this far and changing a habit would be a very time costly option.

Since I've last posted it's up to 3,000+ lines now so I'm making way. Wish at least one of my friends knew VB... Lol my personal dead line is a year so I could use all the help I can get.

Thanks for all the advice!

Architecturally I think you are building yourself a quicksand pit. One level of nested classes can be managable but when you nest 2 and 3 levels deep you can end up in real trouble.

There are many reasons to divide your work into multiple classes, but in a large-scale application the goal is composability. Composability gives you flexibility and testability. Implementation of proper composability requires practicing inversion of control: rather than the car creating its engine a composable system's car expects to be given an engine it can use.

Along with Inversion of Control a good system respects the Open Closed Principle: a class should be open for extensibility and closed for modification. What's that mean? It means the ideal class will be written once. If the car's engine takes gasoline but you'd rather use a diesel, the ideal car design shouldn't care that it's getting a different engine. After all, the engine just provides torque for the drive shaft. Yet without IoC letting you pop in a new engine, most people's approach would be an IsDiesel property on the car that controls which engine it creates. When you want to add a hybrid engine, you have to modify the car again. That's closed to extensibility and open to modification. It leads to sadness.

I can't really suggest a better approach because the level of nesting combined with the missing code is making it difficult for me to envision what I'm reading. But what I'm looking at is a collection of tightly coupled controls. For UI that's not too bad, but where's the logic? What do you do if you need to add a toolbar button? Where's that logic go? If it needs to interact with some other control, how does it get to that control? You mentioned this is a view, where's the controllers/model?

It's probably something that you can make work, but I don't like it. I suggest looking at the SOLID principles created by Robert C. Martin and studying Presentation Model patterns as described by Martin Fowler. Right now everything looks like design patterns for the sake of design patterns and I'm not sold that you need this level of abstraction. Each abstraction is complexity; you shouldn't introduce complexity unless you need it. If there's no way to swap out one AppointmentBookControlTheme for another without modifying AppointmentBookControl then you gain nothing by having that class. 3,000 lines of code shouldn't have enough complexity to be unmanageable yet; right now it looks like 50% of your code is class creation overhead anyway. Don't bother with high falutin' design patterns until you've got enough code to make the complexity overhead negligible.

If you have nested classes (that is, classes which are private to the container - friend or public classes should not be exposed by a class, but should reside in a namespace) you can use 'partial' classes to split your parent class across multiple files. This may ease management of the code.

Quote:

---

Originally Posted by techgnome

... not to discourage you or anything but 2000 lines for a project isn't much...

---

Agreed - and with VB.NET it's easy enough to have a huge project that's extremely easy to overlook with various tools like seperate files, regions and comments. I remember the good old days when programming was done in machine code with many thousand lines of coding, no commenting and nothing even close to the editing capabilities present today; not to mention common practises like self-modifying programs :rolleyes:. Or rung programming on a PLC. Thank god those days are over.

#EDIT: Not to be completely off topic: You (the OP) should most definately start using #Regions!

Quote:

---

Originally Posted by Sitten Spynne

Sense

---

Dammit, so many of your posts need Rep-ping but I have to spread the lovin' more before I can rep you again. :mad:

Quote:

---

Originally Posted by ThomasJohnsen

You (the OP) should most definately start using #Regions!

---

I agree 100%.

Regions are hit and miss with a lot of people. Most of the people smarter than me I follow hate them.

The going theory on the bleeding edge right now is the average class should be relatively small and each method inside should do one thing and be able to pull it off in around a dozen lines. This theory comes from the people who are writing automated tests for everything they do; unit testing works better when your units are tiny. In this case, regions just clutter up things. The "S" in SOLID (now I've covered all but L and D!) is the Single Responsibility Principle: every class should have one and only one responsibility. If a class has only one responsibility, the need for a region diminishes. It's OK to do 2 things if you delegate that 2nd thing to another class.

Personally I don't use regions much. Once I used them to separate the logical parts of classes. Now I take that as the cue for where I need to split a class into helpers. Once I used regions like "Public Methods", "Overridable Methods", etc. But since my classes are sorted the same way every time it was just redundant. The only time I really use them anymore is for explicit interface implementation in C# since that's supposed to be a bit hidden anyway.

I use regions when ever I can.... I sort out all my classes into different sections, public, private, friend, static, properties, constants, methods, etc.... and create a region for each section, EXCEPT event handlers... since Vs wants to put those at the bottom of form classes, I usually jsut let them fall where they want and region out the rest of the class above it. Now, I only do this for the more complex cases... for simple ones, I leave it alone. I make heavy use of the outlining feature, and collapse everything.... when I'm working on a large class, and I need to find the property of XYZ, I know that I can open the "Public Properties" region and easily find it there.

Why don't I jsut "right-click" and select "find definition" ??? because I'm not always in a spot where the property is being used. Why don't I just drop down the list of methods, properties etc and select it from the drop down? I've worked on som really complex systems where there is easily 100's of objects on the form and countless other methods. Using regions is a great way to sort out the chaff from the wheat of what I'm working on.

Not to mention that when I collapse a region, close VS, re-open it, the region is still collapsed. Sweet.

-tg

Quote:

---

Originally Posted by Sitten Spynne

Regions are hit and miss with a lot of people. Most of the people smarter than me I follow hate them.

The going theory on the bleeding edge right now is the average class should be relatively small and each method inside should do one thing and be able to pull it off in around a dozen lines. This theory comes from the people who are writing automated tests for everything they do; unit testing works better when your units are tiny. In this case, regions just clutter up things. The "S" in SOLID (now I've covered all but L and D!) is the Single Responsibility Principle: every class should have one and only one responsibility. If a class has only one responsibility, the need for a region diminishes. It's OK to do 2 things if you delegate that 2nd thing to another class.

Personally I don't use regions much. Once I used them to separate the logical parts of classes. Now I take that as the cue for where I need to split a class into helpers. Once I used regions like "Public Methods", "Overridable Methods", etc. But since my classes are sorted the same way every time it was just redundant. The only time I really use them anymore is for explicit interface implementation in C# since that's supposed to be a bit hidden anyway.

---

I agree with this. This is generally what I follow. No regions, as SS said.

vb.net Code:

'//[Class1.vb]
Partial Public Class Class1
 
    '//delegates
 
    '//events
 
    '//constants
 
    '//fields
 
    '//properties
 
    '//methods
 
    '//event handlers
 
    '//nested types
    Partial Protected Class NestedClass1
        '//no code goes here
    End Class
 
End Class
 
'//[Class1.NestedClass1.vb]
Partial Public Class Class1
 
    '//no code goes here
 
    Partial Protected Class NestedClass1
        '//implementation goes here
    End Class
 
End Class

Quote:

---

Originally Posted by Sitten Spynne

Regions are hit and miss with a lot of people. Most of the people smarter than me I follow hate them.

---

Having read quite a few of your postings, and considering you to be quite a bit smarter than me (nothing sarcastic), the people smarter than you must then logically be smarter than me. But that dosen't mean that I'll stop using Regions :).

Regions truly help me - even in smaller classes to structure the coding. And I find that understanding other people's projects or revisiting very old projects of my own, is so much faster when regions are applied. But it's all a matter of taste, and far be it from me to try to convince you to apply my style when you obviously are a much better programmer than I.

But the OP has (by his own words) 3000+ lines in a single file with sparse comments, multiple layers of nested classes and no regions. I would never venture into reading or trying to comprehend all that coding without a substantial paycheck :sick:. #EDIT: And I'm glad he is not my colleague working with me on a larger scale project.

Tom

Eh, I don't judge people that use regions. Every now and then I try them out again and it feels nice, then I get in a situation where I'm doing as much work keeping the regions up-to-date as I am writing code and I kill them all. A couple months later I ask, "Why am I not using regions?" and it starts again. In the end I find it as obvious where things belong in this case:

Code:

---

Class CashRegister
Class MoneyTill
Class TaxCalculator
Class Item
Class Order

---

as I do in this case:

Code:

---

Class Store
  #Region "Money Till Management"
  #Region "Checkout Register Management"
  #Region "Tax Calculation"
  #Region "Inventory"
  #Region "Subtotals"
  #Region "Total Calculation"
End Class

---

The biggest difference comes when it's time to prove my code is correct. It's easy to test that MoneyTill requests more cash when it's low: set it up with cash, deduct some, see if it raised an event. Doing the same with the store might involve setting up a cash register, creating some inventory items, adding the items to an order, calculating the subtotal, applying tax, paying the register some large sum, then distributing a large amount of change. The harder it is to write the test, the less likely I'll write good ones.

3,000 line files are certainly helped by regions. But it's as helpful as opening a window to dissipate the smell of a full garbage bin. The appropriate solution to stinky garbage is to remove it from your house. So, too, is the appropriate solution to a 3,000 line file to remove what doesn't belong. "Where does this code belong?" is something a developer should be asking at every line.

Quote:

---

Originally Posted by Sitten Spynne

3,000 line files are certainly helped by regions. But it's as helpful as opening a window to dissipate the smell of a full garbage bin. The appropriate solution to stinky garbage is to remove it from your house. So, too, is the appropriate solution to a 3,000 line file to remove what doesn't belong. "Where does this code belong?" is something a developer should be asking at every line.

---

I think the fact that I like regions, has to do with my age. Having done this for a long time, I tend to get 'caught up' in coding and write long files, where I may have been better off splitting the code into multiple files. I blame ANSI C :p.
Object-oriented programming was something completely new, when I got my degree, and today I regret that I didn't put more time into learning proper coding techniques with regards to especially that. In those days however everyone wrote in machine code or ANSI C.

Quote:

---

Originally Posted by Sitten Spynne

Architecturally I think you are building
I can't really suggest a better approach because the level of nesting combined with the missing code is making it difficult for me to envision what I'm reading. But what I'm looking at is a collection of tightly coupled controls. For UI that's not too bad, but where's the logic? What do you do if you need to add a toolbar button? Where's that logic go? If it needs to interact with some other control, how does it get to that control? You mentioned this is a view, where's the controllers/model?

---

Yes I agree with all of this, and this is exactly why I had first stated I would not be able to post my code, and if I had to then it would have to butcher down greatly to where it almost doesn't make sense. Cause like you said it's missing all the guts. If you saw the code then you would have all your questions answered. Each of those divisions I have like toolbar has it's own initializecomponent sub routine for example. Which is called when the user interface is initialized.

Well if I can't see the code or the architectural plan it's hard to give advice. From a pure "this is my UI code" perspective there's nothing (other than the nested classes) that's inherently evil. But the interesting (and difficult) part of software design is the glue between the layers.

Here let me try this..

vb Code:

Option Explicit On
Option Strict On
Option Compare Text
Option Infer
 
Imports System
 
Imports System.ComponentModel
Imports System.ComponentModel.Design
 
Imports System.Drawing
Imports System.Drawing.Drawing2D
Imports System.Drawing.Design
 
Imports System.IO
 
Imports System.Data
Imports System.Data.OleDb
Imports System.Data.Sql
 
Public Class AppointmentBookControl
    Inherits System.Windows.Forms.UserControl
 
    #Region "Main Properties"
 
    <Description(AppointmentBookControlResources.MonthlyOverviewSliderDescription)> _
    Public Property ShowMonthlyOverviewSlider() As Boolean
        Get
            Return Me.Theme.ShowMonthlyOverviewSlider
        End Get
        Set(ByVal value As Boolean)
            Me.Theme.ShowMonthlyOverviewSlider = value
            Me.UserInterface.MonthlyOverviewSliderVisibleChange(value)
            Me.Invalidate()
        End Set
    End Property
 
    <Description(AppointmentBookControlResources.MonthlyOverviewSliderDockDescription)> _
    Public Property MonthlyOverviewSliderDock() As DockStyle
        Get
            Return Me.Theme.MonthlyOverviewSliderDock
        End Get
        Set(ByVal value As DockStyle)
            Me.Theme.MonthlyOverviewSliderDock = value
            Me.UserInterface.MonthlyOverviewSliderDockChange(value)
            Me.Invalidate()
        End Set
    End Property
 
    ' Boarder for now has some known issues.  First and mostly the containers and objects are drawn overtop boarder.
    ' Also boarder thickness calculation needs to be tweeks to display adjusted width correctly.
    <Browsable(False)> _ 
    Public Property ShowBoarder() As Boolean
        Get
            Return Me.Theme.ShowBoarder
        End Get
        Set(ByVal value As Boolean)
            Me.Theme.ShowBoarder = value
            Me.Invalidate()
        End Set
    End Property
 
    Private _Year As Integer = Today.Year
    Public Property Year As Integer
        Get
            Return Me._Year
        End Get
        Set(ByVal value As Integer)
            Me._Year = value
        End Set
    End Property
 
    Private _Month As Integer = Today.Month
    Public Property Month As Integer
        Get
            Return Me._Month
        End Get
        Set(ByVal value As Integer)
            Me._Month = value
        End Set
    End Property
 
    #End Region
 
    Private UserInterface As New AppointmentBookControlUserInterface(Me)
    Private Theme As New AppointmentBookControlTheme(Me)
 
 
    #Region "Constructors"
 
    Sub New()
        ' This call is required by the designer.
        InitializeComponent()
        
        ' Add any initialization after the InitializeComponent() call.
    End Sub
 
    'UserControl overrides dispose to clean up the component list.
    <System.Diagnostics.DebuggerNonUserCode()> _
    Protected Overrides Sub Dispose(ByVal disposing As Boolean)
        Try
            If disposing AndAlso components IsNot Nothing Then
                components.Dispose()
            End If
        Finally
            MyBase.Dispose(disposing)
        End Try
    End Sub
 
    'Required by the Windows Form Designer
    Private components As System.ComponentModel.IContainer
 
    'NOTE: The following procedure is required by the Windows Form Designer
    'It can be modified using the Windows Form Designer.  
    'Do not modify it using the code editor.
    <System.Diagnostics.DebuggerStepThrough()> _
    Private Sub InitializeComponent()
        Me.SuspendLayout()
        '
        'AppointmentBookControl
        '
        Me.AutoScaleDimensions = New System.Drawing.SizeF(6.0!, 13.0!)
        Me.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font
        Me.DoubleBuffered = True
        Me.SetStyle(System.Windows.Forms.ControlStyles.UserPaint Or _
                    System.Windows.Forms.ControlStyles.AllPaintingInWmPaint Or _
                    System.Windows.Forms.ControlStyles.ResizeRedraw Or _
                    System.Windows.Forms.ControlStyles.OptimizedDoubleBuffer, True)
        Me.UpdateStyles()
        Me.Name = "AppointmentBookControl"
        Me.ResumeLayout(False)
    End Sub
 
    Protected Overrides Sub Finalize()
        MyBase.Finalize()
    End Sub
 
    #End Region
 
    Protected Overrides Sub OnLoad(ByVal e As System.EventArgs)
        MyBase.OnLoad(e)
        UserInterface.OnLoad(e)
        Me.Invalidate()
    End Sub
 
    Protected Overrides Sub OnPaint(ByVal e As System.Windows.Forms.PaintEventArgs)
        MyBase.OnPaint(e)
        Theme.OnPaint(e) ' Paint theme.
    End Sub
 
    ' Nested classes.
    Private Class AppointmentBookControlUserInterface
 
        <Browsable(False)> _
        Private Property Owner As AppointmentBookControl
 
        Private Toolbar As AppointmentBookControlToolbar
        Private DaySelectionbar As AppointmentBookControlDaySelectionBar
        Private MonthlyOverviewSlider As AppointmentBookControlMonthlyOverviewSlider
        Private ScheduleGridBody As AppointmentBookControlScheduleGridBody
 
 
        Sub New(ByRef Owner As AppointmentBookControl)
            MyBase.New()
            Me.Owner = Owner ' Pass on the reference.
            Toolbar = New AppointmentBookControlToolbar(Owner)
            DaySelectionbar = New AppointmentBookControlDaySelectionBar(Owner)
            MonthlyOverviewSlider = New AppointmentBookControlMonthlyOverviewSlider(Owner)
            ScheduleGridBody = New AppointmentBookControlScheduleGridBody(Owner)
        End Sub
 
        Public Sub OnLoad(ByVal e As System.EventArgs)
            ScheduleGridBody.InitializeComponent()
            DaySelectionbar.InitializeComponent()
            Toolbar.InitializeComponent()
            MonthlyOverviewSlider.InitializeComponent()
        End Sub
 
        Public Sub MonthlyOverviewSliderVisibleChange(ByVal Visible As Boolean)
            MonthlyOverviewSlider.Container.Visible = (Visible)
        End Sub
 
        Public Sub MonthlyOverviewSliderDockChange(ByVal DockStyle As DockStyle)
            MonthlyOverviewSlider.Container.Dock = DockStyle
        End Sub
 
        ' Nested classes for each of the four user interface divisions.
        Private Class AppointmentBookControlToolbar
 
            <Browsable(False)> _
            Private Property Parent() As AppointmentBookControl
 
            Private ReadOnly Property Skin() As AppointmentBookControlTheme.AppointmentBookControlThemeSkin
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Skin
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private ReadOnly Property [Region]() As Rectangle
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Structures.ToolBarRect
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private WithEvents Container As New AppointmentBookControl.AppointmentBookControlPanelControl
            Private WithEvents Button1 As New AppointmentBookControl.AppointmentBookControlButtonControl
            Private WithEvents Button2 As New AppointmentBookControl.AppointmentBookControlButtonControl
 
 
            #Region "Constructor"
 
            Sub New(ByRef Parent As AppointmentBookControl)
                MyBase.New()
                Me.Parent = Parent ' Pass on the reference.
            End Sub
 
            Public Sub InitializeComponent()
                Me.Parent.SuspendLayout()
                '
                ' Container
                '
                Me.Parent.Controls.Add(Container)
                With Container
                    .Width = [Region].Width
                    .Height = [Region].Height
                    .Location = New Point([Region].Left, [Region].Top)
                    .Dock = DockStyle.Top
                    '.BackColor = color.Red ' < Indicator
                End With
                '
                ' Button 1
                '
                Me.Container.Controls.Add(Button1)
                With Button1
                    .Location = New Point(8, 8)
                    .Size = New Size(Container.Height*2-16, Container.Height-16)
                    .Curvature = 4
                    .Name = "ButtonControl1"
                    .Caption = "Button 1"
                End With
                '
                ' Button 2
                '
                Me.Container.Controls.Add(Button2)
                With Button2
                    .Location = New Point(Button1.Width+16, 8)
                    .Size = New Size(Container.Height*2-16, Container.Height-16)
                    .Curvature = 4
                    .Name = "ButtonControl2"
                    .Caption = "Button 2"
                End With
                Me.Parent.ResumeLayout(True)
            End Sub
 
            #End Region
 
            Public Sub OnPaint(ByVal e As System.Windows.Forms.PaintEventArgs)
                Dim Graphic As Graphics = e.Graphics
 
                Graphic.TranslateTransform([Region].Left, [Region].Top)
                Try
                    Graphic.DrawString("Toolbar", Skin.Font, Brushes.Black, New Point(0, 0))
                Catch ex As Exception
                Finally
                    Graphic.ResetTransform()
                    Graphic = Nothing
                End Try
            End Sub
 
            Private Sub Button1_Click(ByVal sender As Object, ByVal e As System.EventArgs) Handles Button1.Click
                'Beep
            End Sub
 
        End Class
 
    End Class
 
End Class

Post 1:

Here let me try this..

vb Code:

Option Explicit On
Option Strict On
Option Compare Text
Option Infer
 
Imports System
 
Imports System.ComponentModel
Imports System.ComponentModel.Design
 
Imports System.Drawing
Imports System.Drawing.Drawing2D
Imports System.Drawing.Design
 
Imports System.IO
 
Imports System.Data
Imports System.Data.OleDb
Imports System.Data.Sql
 
Public Class AppointmentBookControl
    Inherits System.Windows.Forms.UserControl
 
    #Region "Main Properties"
 
    <Description(AppointmentBookControlResources.MonthlyOverviewSliderDescription)> _
    Public Property ShowMonthlyOverviewSlider() As Boolean
        Get
            Return Me.Theme.ShowMonthlyOverviewSlider
        End Get
        Set(ByVal value As Boolean)
            Me.Theme.ShowMonthlyOverviewSlider = value
            Me.UserInterface.MonthlyOverviewSliderVisibleChange(value)
            Me.Invalidate()
        End Set
    End Property
 
    <Description(AppointmentBookControlResources.MonthlyOverviewSliderDockDescription)> _
    Public Property MonthlyOverviewSliderDock() As DockStyle
        Get
            Return Me.Theme.MonthlyOverviewSliderDock
        End Get
        Set(ByVal value As DockStyle)
            Me.Theme.MonthlyOverviewSliderDock = value
            Me.UserInterface.MonthlyOverviewSliderDockChange(value)
            Me.Invalidate()
        End Set
    End Property
 
    ' Boarder for now has some known issues.  First and mostly the containers and objects are drawn overtop boarder.
    ' Also boarder thickness calculation needs to be tweeks to display adjusted width correctly.
    <Browsable(False)> _ 
    Public Property ShowBoarder() As Boolean
        Get
            Return Me.Theme.ShowBoarder
        End Get
        Set(ByVal value As Boolean)
            Me.Theme.ShowBoarder = value
            Me.Invalidate()
        End Set
    End Property
 
    Private _Year As Integer = Today.Year
    Public Property Year As Integer
        Get
            Return Me._Year
        End Get
        Set(ByVal value As Integer)
            Me._Year = value
        End Set
    End Property
 
    Private _Month As Integer = Today.Month
    Public Property Month As Integer
        Get
            Return Me._Month
        End Get
        Set(ByVal value As Integer)
            Me._Month = value
        End Set
    End Property
 
    #End Region
 
    Private UserInterface As New AppointmentBookControlUserInterface(Me)
    Private Theme As New AppointmentBookControlTheme(Me)
 
 
    #Region "Constructors"
 
    Sub New()
        ' This call is required by the designer.
        InitializeComponent()
        
        ' Add any initialization after the InitializeComponent() call.
    End Sub
 
    'UserControl overrides dispose to clean up the component list.
    <System.Diagnostics.DebuggerNonUserCode()> _
    Protected Overrides Sub Dispose(ByVal disposing As Boolean)
        Try
            If disposing AndAlso components IsNot Nothing Then
                components.Dispose()
            End If
        Finally
            MyBase.Dispose(disposing)
        End Try
    End Sub
 
    'Required by the Windows Form Designer
    Private components As System.ComponentModel.IContainer
 
    'NOTE: The following procedure is required by the Windows Form Designer
    'It can be modified using the Windows Form Designer.  
    'Do not modify it using the code editor.
    <System.Diagnostics.DebuggerStepThrough()> _
    Private Sub InitializeComponent()
        Me.SuspendLayout()
        '
        'AppointmentBookControl
        '
        Me.AutoScaleDimensions = New System.Drawing.SizeF(6.0!, 13.0!)
        Me.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font
        Me.DoubleBuffered = True
        Me.SetStyle(System.Windows.Forms.ControlStyles.UserPaint Or _
                    System.Windows.Forms.ControlStyles.AllPaintingInWmPaint Or _
                    System.Windows.Forms.ControlStyles.ResizeRedraw Or _
                    System.Windows.Forms.ControlStyles.OptimizedDoubleBuffer, True)
        Me.UpdateStyles()
        Me.Name = "AppointmentBookControl"
        Me.ResumeLayout(False)
    End Sub
 
    Protected Overrides Sub Finalize()
        MyBase.Finalize()
    End Sub
 
    #End Region
 
    Protected Overrides Sub OnLoad(ByVal e As System.EventArgs)
        MyBase.OnLoad(e)
        UserInterface.OnLoad(e)
        Me.Invalidate()
    End Sub
 
    Protected Overrides Sub OnPaint(ByVal e As System.Windows.Forms.PaintEventArgs)
        MyBase.OnPaint(e)
        Theme.OnPaint(e) ' Paint theme.
    End Sub
 
    ' Nested classes.
    Private Class AppointmentBookControlUserInterface
 
        <Browsable(False)> _
        Private Property Owner As AppointmentBookControl
 
        Private Toolbar As AppointmentBookControlToolbar
        Private DaySelectionbar As AppointmentBookControlDaySelectionBar
        Private MonthlyOverviewSlider As AppointmentBookControlMonthlyOverviewSlider
        Private ScheduleGridBody As AppointmentBookControlScheduleGridBody
 
 
        Sub New(ByRef Owner As AppointmentBookControl)
            MyBase.New()
            Me.Owner = Owner ' Pass on the reference.
            Toolbar = New AppointmentBookControlToolbar(Owner)
            DaySelectionbar = New AppointmentBookControlDaySelectionBar(Owner)
            MonthlyOverviewSlider = New AppointmentBookControlMonthlyOverviewSlider(Owner)
            ScheduleGridBody = New AppointmentBookControlScheduleGridBody(Owner)
        End Sub
 
        Public Sub OnLoad(ByVal e As System.EventArgs)
            ScheduleGridBody.InitializeComponent()
            DaySelectionbar.InitializeComponent()
            Toolbar.InitializeComponent()
            MonthlyOverviewSlider.InitializeComponent()
        End Sub
 
        Public Sub MonthlyOverviewSliderVisibleChange(ByVal Visible As Boolean)
            MonthlyOverviewSlider.Container.Visible = (Visible)
        End Sub
 
        Public Sub MonthlyOverviewSliderDockChange(ByVal DockStyle As DockStyle)
            MonthlyOverviewSlider.Container.Dock = DockStyle
        End Sub
 
        ' Nested classes for each of the four user interface divisions.
        Private Class AppointmentBookControlToolbar
 
            <Browsable(False)> _
            Private Property Parent() As AppointmentBookControl
 
            Private ReadOnly Property Skin() As AppointmentBookControlTheme.AppointmentBookControlThemeSkin
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Skin
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private ReadOnly Property [Region]() As Rectangle
                Get
                    If Parent IsNot Nothing Then
                        Return Me.Parent.Theme.Structures.ToolBarRect
                    Else Return Nothing
                    End If
                End Get
            End Property
 
            Private WithEvents Container As New AppointmentBookControl.AppointmentBookControlPanelControl
            Private WithEvents Button1 As New AppointmentBookControl.AppointmentBookControlButtonControl
            Private WithEvents Button2 As New AppointmentBookControl.AppointmentBookControlButtonControl
 
 
            #Region "Constructor"
 
            Sub New(ByRef Parent As AppointmentBookControl)
                MyBase.New()
                Me.Parent = Parent ' Pass on the reference.
            End Sub
 
            Public Sub InitializeComponent()
                Me.Parent.SuspendLayout()
                '
                ' Container
                '
                Me.Parent.Controls.Add(Container)
                With Container
                    .Width = [Region].Width
                    .Height = [Region].Height
                    .Location = New Point([Region].Left, [Region].Top)
                    .Dock = DockStyle.Top
                    '.BackColor = color.Red ' < Indicator
                End With
                '
                ' Button 1
                '
                Me.Container.Controls.Add(Button1)
                With Button1
                    .Location = New Point(8, 8)
                    .Size = New Size(Container.Height*2-16, Container.Height-16)
                    .Curvature = 4
                    .Name = "ButtonControl1"
                    .Caption = "Button 1"
                End With
                '
                ' Button 2
                '
                Me.Container.Controls.Add(Button2)
                With Button2
                    .Location = New Point(Button1.Width+16, 8)
                    .Size = New Size(Container.Height*2-16, Container.Height-16)
                    .Curvature = 4
                    .Name = "ButtonControl2"
                    .Caption = "Button 2"
                End With
                Me.Parent.ResumeLayout(True)
            End Sub
 
            #End Region
 
            Public Sub OnPaint(ByVal e As System.Windows.Forms.PaintEventArgs)
                Dim Graphic As Graphics = e.Graphics
 
                Graphic.TranslateTransform([Region].Left, [Region].Top)
                Try
                    Graphic.DrawString("Toolbar", Skin.Font, Brushes.Black, New Point(0, 0))
                Catch ex As Exception
                Finally
                    Graphic.ResetTransform()
                    Graphic = Nothing
                End Try
            End Sub
 
            Private Sub Button1_Click(ByVal sender As Object, ByVal e As System.EventArgs) Handles Button1.Click
                'Beep
            End Sub
 
        End Class
 
    End Class
 
End Class

Post 2:

Code was too long so I had to split things up...

Here is my main class, with ONE of SEVEN nested classes. Within that class are 3 more nested classes. One included is ONE of FOUR "user interface" classes I broke down and constructed. The one I have included is the toolbar specifically, which handles anything needed to display in the toolbars region. Now this code doesn't include the Theme class which handles everything you see and the regions of "user interface" locations and sizes.

Again I fallow very particular coding methods of my preference so every other class tends to fallow the same pattern you see here. Such as the three "user interfaces". They all have similar properties like parent, and constructors like new and initialize components. Then all else is done as normal like handing events and such.

By the way, ThomasJohnsen, this isn't a bragging contest. I'm asking for help so I ask if you stay on topic please. The reason for me posting how lengthy my project was to inform those who actually want to help know what there getting into.

Ok so I just realized the code I posted was an older version which isn't much help, if I could I would delete them but give me till tomorrow and I'll upload my updated code seeing as a left my thumb drive at work.

rather than posting the code in post after post after post.... just zip it up, remove any compiled code (dll & exe) and then upload that... then anyone that's interested in looking at it, can download it, tinker, and upload it back...

-tg

You asked for advice about your design. Our opinion is it is needlessly complex. We're not bragging. 3,000 lines of code is quite trivial; I'm responsible for more than 250,000 lines of code daily. But 3,000 lines before you even have any functional code? That's a sign of architecture astronomy. Sometimes advice hurts. When you ask, "How can I improve this?" the answer is not always, "Wow! This is the best design I've ever seen!"

Size is one of the easiest to understand measures of the complexity of code. The most elegantly designed 1,000,000 line codebase is still more complicated than a 10,000 line ball of mud. The other things he mentioned in #17 (sparse comments and high degree of nesting) contribute to this overcomplexity. I really think you've got too much cruft and you've overengineered this design. I'm especially certain of this since you've demonstrated no logic; you've demonstrated a lot of UI but no functionality.

What the heck kind of project are you writing that you up-front know you need 52 views? That's usually reserved for the kind of project where you spend months planning the architecture in advance. Can you draw some relationship diagrams to show how everything fits together? If not, that's a problem. This complexity is killing you and ignoring it while you "get things done" is making it worse. Don't write another line until you don't feel crushed by complexity.

I can tell you don't even have source control in place, since you can't seem to figure out which version to paste in the forums. You need to see to that. If I had 52 views that did nothing, I'd expect 100,000+ lines of support code to back it up. Zip files with dates on them aren't version control. You need Git, Mercurial, SVN, even CVS to help you keep track of the changes you're making. You need automated tests to help you understand that the moving parts of your program function well enough that the integration should be easy. You probably need integration tests to make sure that works. A continuous integration system couldn't hurt either. 52 views is an enterprise-level system; if you aren't using enterprise-level methodologies it's going to become a quagmire.

Or maybe you need all this stuff and haven't articulated why. To date, all you've done is tell us there's a big mess behind a fence but you've only given us a tiny hole to peep through, occasionally retracting the view because it was the wrong version. Zip up your project and post it. Or produce some architectural diagrams. Or post a high-level description of what you're doing and how you've tried to do it. That's the only way anyone's going to be able to help you. I cannot tell you how to restructure a form without understanding what it's supposed to do and how it fits with the logic that drives it. I cannot tell you how to manage 52 views with no clue as to what you call a view, why there's 52 of them, or what they're even supposed to be doing.

There's a section in Head First Design Patterns that simulates a discussion between a sensei and his student. The discussion illustrates the three phases of a student using design patterns:

1. The student is ignorant of design patterns.
2. The student learns about design patterns and uses them at every opportunity.
3. The master knows when design patterns are appropriate and uses them when required.

I have a feeling you're at 2. I'm still transitioning from 2 to 3. Part of how I learn is to try and help other people learn when to use them; when my examples fall on their face I learn when it wasn't appropriate to use them. Don't use patterns for the sake of using patterns. Every pattern increases one kind of complexity in return for managing another. If you blindly apply patterns, complexity grows exponentially. If you use them to control the worst parts of your code, complexity drops dramatically. Right now all I see is patterns; you've provided no justification.

If the reason you aren't posting code is because you think no one is going to bother wading through a massive project like 3,000 lines of code, it's time for you to put your ego on the shelf. It's been a decade since 3,000 lines struck me as particularly large. I bet in 2 hours of analysis I could show you where the worst decisions were made and start talking about a better design. It'd take 2-3 days of my leisure time to say anything concrete; the trick is making me want to spend it. Insulting my intelligence by implying that I won't be able to understand what amounts to less than 1% of the code I handle daily at work sure doesn't motivate me.

If the reason you aren't posting code is because it's your employer's code and you have no right to post it, that's understandable. Come out and say it, quit with the teasing. It's tough to make analogous code, but you really need to provide us a *functional* example that follows your patterns for us to make any suggestions. The code I've seen so far looks overcomplicated, but I can see some justifications that might make me overlook it. To get there, I have to know why you chose the design. So you either have to tell me why you made the decisions or give me a lot of similar code so I can deduce it for myself.

If you're not posting code because you're worried about someone stealing your ideas, I don't think that's well-founded. If you've got something worth that much, hire some developers that know what they're doing to write it. You can write employment contracts that make sure you own the code they write; technically every character I type is owned by my company, even the stuff I do at home. The worst thing you can do is continue along without guidance and let someone else beat you to execution. Good products ship. Winning the killer app game involves shipping before the other guy. While you're wasting time trying to get help for free, someone else is working on exactly what you're working on. Will you beat them to market? I bet not. Even if you do, will you be able to push out new features faster than them? If you don't understand your architecture, you won't.

So provide some details already. I'm tired of licking drops of gravy from the plate. I want some meat.

Quote:

---

Originally Posted by Sitten Spynne

I can tell you don't even have source control in place, since you can't seem to figure out which version to paste in the forums. [...] You need Git, Mercurial, SVN, even CVS to help you keep track of the changes you're making.
[...]
To date, all you've done is tell us there's a big mess behind a fence but you've only given us a tiny hole to peep through, occasionally retracting the view because it was the wrong version. Zip up your project and post it. Or produce some architectural diagrams. Or post a high-level description of what you're doing and how you've tried to do it.

---

Or, kill two birds with one stone and get yourself an account at a public source repository hosting such as GitHub etc. For a public repository (anyone can see the code), these are generally free - then we'd be able to see the whole of your project easily. You'd have to accept what Sitten Spynne says about publishing your code though. If you don't accept that, you can pay for an account that has private repositories (i.e. only people you authorise have access to the source code).

Alright here's what you asked for, I zipped things up and here is my full project as is. I would also like to apologize for any confusion that I may have caused, seeing as I'm new here, and new to the forum world. I may not know everything or how to get my point across correctly the first time but all I ask for is patience, I'll catch up quick.

Attached to this post is my project. For anyone willing to over look it then be my guest.

Again I'd like to say that my original intentions for posting this is to correct any of my bad habits that exist now before I continue and kick this project into full gear. This example includes a lot of the logic that was missing before, and the complete class structure. Also the master control contains several sub controls which are used for a more custom purpose. Visually more than anything. The code is 100% mine. However my intentions for this project is for it to be part of a much larger project. This is something I've been working on for some time, while simultaneously becoming acquainted to .NET. Seeing that my native speak was vb6, lots of these new functions .NET supply for us weren't there back in the day and I know for a fact I'm not using or even know half of them.

Again thanks for your time and patience.

Squee! I love looking over projects, especially any form of custom control. My hobby *and* career involves custom controls. I don't think there's much chance I'll have useful commentary before Thursday evening ("evening" meaning in the US Central time zone) but this looks pretty solid. It's not as bad as I thought, but I think the nested classes make it harder to understand the pieces and how they fit together. I'll separate them and see if that exposes any coupling problems.

I am a ruthless code reviewer and my coworkers will back it up. My advice is sometimes interpreted at scolding. I'm scolding the code, not the person.For some things (like the Catch ex As Exception I'm going to rant about later) I want to make it really clear that it's a bad practice. The best way to do so is harshly condemn the practice. You're not stupid for using these mechanisms; it's Microsoft's fault for making the wrong thing the obvious solution. Generally, the more angry at a practice I sound the more frequently you can tell people new to VB .NET fall into the trap. I'm not angry at you or your code, but instead at the framework designers that not only left a pot of boiling water where children could reach it but also dangled candy from the pot's handles Sometimes it's not .NET's fault but is instead a silly programming practice that textbooks and other training materials accidentally enforce. I wasted 6 years of my life not understanding OOP properly because I read the wrong books; I try really hard to save people from the same fate.

For now, I have only a little bit of advice after spending a couple of minutes looking it over (I spent far less time on analysis than these comments.)

• You don't need the Methods module at all; I'll explain each one.
• IIf serves to help you cram as much as possible on one line: this is the enemy of readable code. If I only gave you 15 seconds to explain what a code snippet does, which do you think you could do?
  
  Code:

  ---

  Return New Rectangle(New Point(0, 0), New Size(Parent.Width, IIf(Me.Parent.ShowColumnImages, 96, 32)))

  ---

  Code:

  ---

  Dim width As Integer = Parent.Width
Dim height As Integer = Parent.ColumnHeight
Return New Rectangle(0, 0, width, height)

  ---

  I made up a property for that; I'm not certain the property belongs there but doesn't it make those three lines a heck of a lot easier to understand than the one? Not to mention if you need to debug you have a fighting chance of getting help.
  
• DaysInMonth() is redundant and incorrect; years divisible by 100 are only leap years if they are also divisible by 400. For example, 2000 was a leap year but 2100 will not be. DateTime.DaysInMonth() is already implemented and implemented correctly for years 1 AD through 9999 AD, assuming the calculation algorithm doesn't change. If it *does* change, MS will update it and your code will still work. (Don't interpret this as a scolding; you probably didn't know it existed. Before you write a common utility method like this, spend a few minutes looking to see if it already exists or just ask on a forum.)
  
• IsToday() is trivial enough you don't need it with some tweaks. Don't store date and time as three variables; use the DateTime structure. Then you can avoid writing code to determine if the date is Today. For example, here's line 359 where it's used twice:
  
  Code:

  ---

  Graphic.DrawString(str, _
                    IIf(IsToday(.Year, .Month, i), Me.Skin.DaySelectionBarBoldFont, Me.Skin.DaySelectionBarRegularFont), _
                    IIf(IsToday(.Year, .Month, i), SolidBoldFontBrush, SolidDimFontBrush), _
                    New PointF(Convert.ToSingle(8+a.Width+(p*(i-1))), _
                                Convert.ToSingle([Region].Height/2-s.Height/2)))

  ---

  Here's what it *could* look like:
  
  Code:

  ---

  Dim selectedDate As DateTime = Parent.UserInterface.ScheduleGridBody.SelectedDate
Dim dayBarFont As Font = Skin.DaySelectionBarRegularFont
Dim fontBrush As Brush = SolidDimFontBrush
If (selectedDate.Date = DateTime.Today) Then
    dayBarFont = Skin.DaySelectionBarBoldFont
    fontBrush = SolidBoldFontBrush
End If
Dim x As Single = CSng(8 + a.Width + (p * (i - 1)))
Dim y As Single = CSng([Region].Height / 2 - s.Height / 2)

Graphic.DrawString(str, dayBarFont, fontBrush, new PointF(x, y)

  ---

• (Still on the last point but a separate idea.) The focus should be on "What does this line do?". You type code once and have to read it multiple times. That means saving 100 keystrokes today means nothing if it costs you 10 seconds every time you have to ask, "What's this do again?" More lines need not be more complex. I like to write code with too many lines, then prune them as needed. In a real application, I'd probably replace all of the above with a Sub:
  
  Code:

  ---

  DrawDate(index, Graphics)

  ---

  "But that hides what it's doing!" Exactly. I only want to see *how* to draw a date if I'm curious how it's drawn. If I just want to know what's being painted, I'd rather see "Draw the month, draw the year, then draw each day" than the 50 lines it takes to describe how to do that.
• NEVER use "Catch ex As Exception". There are some very rare cases in which it is a bad idea, but until you learn about them "never" is the rule of thumb. "On Error Resume Next" was a nightmare in VB6 and Catch ex As Exception is its counterpart in VB .NET. Either describe the exceptions you know about and can handle and let others fail or don't handle any at all. 9 times out of 10, this bites you when you notice an odd rendering glitch and spend hours inspecting the properties related to it before you later stumble upon the exception handler that causes all of that code to be skipped. Don't shoot yourself in the foot!

I can't stress "optimize for readability" enough. On my first major project, I wrote a 900-line abomination of a method designed to do way too much. I've written 20,000+ line applications that are easier to understand than that method despite being 20x the size. "Just another Perl Hacker" is a great example. Here's a code snippet that forks a child process for every letter in the phrase and uses thread synchronization to force each thread to print its character in the correct order:

Code:

---

@P=split//,".URRUU\c8R";@d=split//,"\nrekcah xinU / lreP rehtona tsuJ";sub p{
@p{"r$p","u$p"}=(P,P);pipe"r$p","u$p";++$p;($q*=2)+=$f=!fork;map{$P=$P[$f^ord
($p{$_})&6];$p{$_}=/ ^$P/ix?$P:close$_}keys%p}p;p;p;p;p;map{$p{$_}=~/^[P.]/&&
close$_}%p;wait until$?;map{/^r/&&<$_>}%p;$_=$d[$q];sleep rand(2)if/\S/;print

---

That's not clever. It's wrong. If I found it in our source tree I'd want to replace the guy who wrote it with one that would write this instead.

The key to good software development: Simplex veri sigillum. "Simplicity is the seal of truth."

I'm assuming that when you wrote "NEVER use "Catch ex As Exception". " ... the rest of that was supposed to read " with nothing in the catch block"... right? because a try catch with nothing in the catch block is the same as OERN... BUT....

catch ex as Exception IS perfectly valid... yes, you SHOULD trap for known more granular exceptions and work your way down to more generic exceptions... but there are cases, where you jsut need to know an exception happened, log it, and move on... even if you trap for 20 specific exceptions... it's that 21st one that will get you... having a generic handler like ex as exception is good, providing you're doing something useful with it... and you're at the bottom level of the stack... I think exception handlers should be at the smallest level possible and that application-level ones are a sign of "I'm too important to go track through all my code and write proper error handlers where they belong.

-tg

Quote:

---

Originally Posted by techgnome

I'm assuming that when you wrote "NEVER use "Catch ex As Exception". " ... the rest of that was supposed to read " with nothing in the catch block"... right? because a try catch with nothing in the catch block is the same as OERN... BUT....

catch ex as Exception IS perfectly valid... yes, you SHOULD trap for known more granular exceptions and work your way down to more generic exceptions... but there are cases, where you jsut need to know an exception happened, log it, and move on... even if you trap for 20 specific exceptions... it's that 21st one that will get you... having a generic handler like ex as exception is good, providing you're doing something useful with it... and you're at the bottom level of the stack... I think exception handlers should be at the smallest level possible and that application-level ones are a sign of "I'm too important to go track through all my code and write proper error handlers where they belong.

-tg

---

The best analogy I can draw to, "I'd like to do this emergency cleanup/logging when an exception I don't understand" is something like, "A hurricane is coming in 2 days; I better make sure my rose bushes are trimmed!" When an exception you didn't expect has wound its way through your code, your application is in an unpredictable state. The only thing you can guarantee is safe is failure.

There are two appropriate places for a catch-all exception handler:

• When writing asynchronous code, the exception only walks the call stack of the worker thread. If you want to propagate that exception the thread that started the asynchronous operation, you have to catch it and shuttle it to the main thread. RunWorkerCompletedEventArgs.Error is a manifestation of this pattern.
• At the highest levels of your code. There's a global "unhandled exception" handler that gets called when your application's in its death rattle. This can still be futile; what if it was OutOfMemoryException? Since the application's dying you don't know what's still alive, so you'd have to create an instance of an object to log. But you don't have the memory to create the log object! It turns out the only way to handle it properly involves creating child AppDomains in which you run the application. Messy. That's watering the garden when doom is bearing down on you.

This is not a use case where it's correct. (I'm referring to line 368 specifically; there may be other, less dangerous instances.) "If a problem happens during rendering, just quit wherever you are." Even if you log it and continue, it leaves the control in a weirdo state that will possibly linger until whatever causes the exception is changed. What if the situation that breaks the control is unsolvable without the pieces that can't be rendered? That's bad for the user and they're going to have to close the app anyway. In my experience, rendering code should never contain something that can throw an exception because the most common cause is "form sizes I didn't think about". Better to think about them. Make sure denominators can't be zero. If something should always be a certain width, double-check it and do something sane if it's not.

I've made controls that change their layout when under space pressure, and I've also enforced minimum sizes using the appropriate properties. Beyond that, I don't mind saying "If you don't give me 100x100 I'm not going to render anything". Letting an exception indicate this is wrong: they are for exceptional circumstances that weren't predicted. It can take 10x longer (or worse) to generate an exception than to just check if the thing that breaks the code is happening. If you know "at 99 pixels this will die", then surround it with "If width < 99 Then Return" and don't let the user resize the form too small (or at least document that if they do so the results are unsatisfactory.) Otherwise you clog up your error log with "errors" that represent the user doing silly things that don't gum up the works. I don't want to have to wade through 100,000 "I tried to render when the width is < 100" when I'm trying to find "NullReferenceException at line 879".

This is why I think it's ridiculous that applications don't auto-save by default. Depending on the exception that's thrown and how bad the situation is, trying to save the data and shutdown gracefully might make things worse. What if you throw because the in-memory data's corrupted? Saving would be destructive! Apple's solving this problem for users with OS-level auto-save and (optionally cloud-backed) version history; if your application uses the standard patterns for saving documents then you don't have to write a line of code to save your user. If only Microsoft would copy their feature!

*edit*
OK, there's a third appropriate place for this mechanism: in debug code it's fine to toss up a message box with some information so long as you address the issue causing the exception. The idea is to explore which things can cause problems, address the ones you care to address, then let the ones you don't address bubble up and potentially crash the application once the release build is made. Don't think release code should have a crashing bug? I don't either. That's why I think about each line I write.

Thanks Sitten Spynne for your reply!

I know it took my a while to post the correct version but that's because I had lost my thumb drive that I carry back and forth from work.. I tend to be good with organizing my project versions and backing up regularly but of course when it slips my mind, things like this happen. Also I guess my biggest reason for not wanting to post my COMPLETE project had to do a little with my self conscience. Afraid of being told more bad news than good..

Custom controls seem to be my thing too. Considering that more than 90&#37; of my projects consist of them.

Now those IIf's I agree with you there. However you see I have multiples of them serving a specific purpose of whats being return. The original Method uses Objects as it's returning parameters. From what I've been told using objects requires a lot more room to store and also requires a conversion from types depending on what your using. So to avoid this I made simple yet specific IIf functions to handle them without using object types.

Again I do realize these can all be avoided as you pointed out with your example and I would actually prefer to do things this way. I probably chose to do so my way as a shortcut to fix a problem.

DaysInMonth() I actually wasn't aware of being incorrect. Also again, this is a perfect example of something that I had been unaware of having been implemented with the .net framework. This will be replaced, fixed, and removed.

The Try, Catch, End Try is a new feature to me as well. This has actually caused me quite some inconvenience in the past. To where I had a single error that I spend days trying to just locate the problem. Once I did, fixing it simple. Turned out that a Try block was hiding the true error. I generally use a message box to display general information of the exception to help me of when the exception occurs, but as I'm beginning to realize this doesn't always help my cause.


So while I wait for your next post at the end of the week I'll briefly review my code and try to apply some of your suggestions so far.

Again I appreciate your time reviewing my project and helping me out as much as you can.

Edit:
In addition regarding your first concern. My class structure is a bit confusing, I know this. But this was exactly why I decided to stop, and seek help. At the rate this project is going things could become irreversibly messy. So I feel my best plan of action was to take a step back, ask for help, review my project for bad habits or incorrectly use of methods, note, double note, then apply.

Quote:

---

Originally Posted by techgnome

catch ex as Exception IS perfectly valid... yes, you SHOULD trap for known more granular exceptions and work your way down to more generic exceptions... but there are cases, where you jsut need to know an exception happened, log it, and move on... even if you trap for 20 specific exceptions... it's that 21st one that will get you... having a generic handler like ex as exception is good, providing you're doing something useful with it... and you're at the bottom level of the stack... I think exception handlers should be at the smallest level possible and that application-level ones are a sign of "I'm too important to go track through all my code and write proper error handlers where they belong.-tg

---

There's two issues here that should not be conflated. One is "catch the most specific exception you can deal with", the other is "never catch System.Exception".

What are you going to do with OutOfMemoryException? StackOverflowException? These ones are why you never catch System.Exception, it's related but not quite the same thing as catching the most specific exception you can.

Now personal preferences:
Catch only what you can do something with. Everything else, leave to bubble up and have an unhandled exception handler installed. (Although I think that might be what you were referring to with your "at the bottom level of the stack" comment?)
Fail fast - an exception you're not prepared for means your application is in an unpredictable state. Better to stop the application, potentially salvage what you can into a structure that the user can optionally recover from (a la Word) and then restart than keep on going potentially corrupting the users data beyond recovery.
Prefer checking that an operation is valid before performing it rather than relying on exception handling (the canonical example here is check a file exists rather than relying on FileNotFoundException).

Quote:

---

Originally Posted by Evil_Giraffe

There's two issues here that should not be conflated. One is "catch the most specific exception you can deal with", the other is "never catch System.Exception".

---

Good note Evil. Again the Try, Catch block is new to me. Still trying to perfect it's use. But your right, there are some exceptions where there would be no point in catching. I only use the general exception because I'm still trying to become familiar with all the exceptions available and when to properly use them. Plus it's a easy fix when I'm trying to get through as much as possible. < bad habit

Wrangling it all down to something manageable would probably take a week or more of concentrated effort; I'm too behind at work to commit to that in any reasonable timeframe. I'll kick around the notion of cleaning it up in free time, but for now I'm just going to make some suggestions now that I've seen the structure of your code.

There's a couple of things making my progress a little slower: one is I don't have a good handle on what's required. Right now clicking on the control doesn't seem to do much; I don't know if that's intended or a bug. Also, your code seems to depend on an Access database for data; it's not provided with the project nor do I have a way to work with Access databases. So I'll have to write a data abstraction layer before moving to late testing. Not appealing.

Needless Classes
Your main control has a nested class for each element of the control. Each of those elements has a nested class that represents the "theme" of the control; this comprises some visual properties and a method to paint the control. Each of those themes has a "skin" class that defines further visual properties. Some of the themes have a "structures" file that seems to define even more properties.

This leads to a confusing mess. When I want to see where a color or bounding box is defined, I have three or four different places to look. I'm sure your goal was to make a very customizable control, but in the end you hurt customization. Since the Theme property isn't a class the designer knows how to support, you can't edit any of its properties at design time. You'd have to do a little extra work to make that happen. Then you'd have to make the Skin property of that theme editable. It'd all be easier if you'd just make these properties of the control itself. There's probably merit to making the elements like the day selection bar individual classes; that supports a UserControl model more than the one you're using.

Gold Plating
Gold plating refers to when we spend time and effort on things that aren't vital. I think you've spent a lot of effort on gold plating.

While you do need to decide on a color for many of these elements, it's probably good enough to expose the property to the designer. Since your classes are nested, it's difficult to create new themes, skins, and structures to customize the appearance. If instead you had all of the properties available on the control (or sub-elements of the control) it'd be a heck of a lot easier to customize the look. Worried it'd be too many properties? Maybe you won't need all of them to be exposed.

The only reason for having these classes is if you're planning on selling this as a control to an audience that has made requests for extraordinary customization. This is a rare use case, and I think you need to study some commercial controls to see how they do it. Our solution is to provide properties for simple things and an OwnerDraw-like model for elements that have customer requests for complex customizations.

Don't do too much on one line.
An example from ButtonControlTheme:

Code:

---

Graphic.DrawString(Me.Parent.Theme.Caption, Me.Skin.Font, FontBrush, New Point(Convert.ToInt32(Me.Parent.Width / 2 - sf.Width / 2), Convert.ToInt32(Me.Parent.Height / 2 - sf.Height / 2)))

---

When you do this, it's very difficult to understand what's happening on the line. What's happening is you're drawing the string centered. So make it more clear:

Code:

---

Dim centerX As Integer = (Me.Parent.Width - sf.Width) \ 2
Dim centerY As Integer = (Me.Parent.Height - sf.Height) \ 2
Dim centerPoint As New Point(centerX, centerY)
Graphic.DrawString(Caption, Skin.Font, FontBrush, centerPoint)

---

Note the "\" operator does Integer division and doesn't return a Double. It's one of VB's handier features.

Don't overqualify properties.
In that last example, you used Me.Parent.Theme.Caption. The class is ButtonControlTheme. If it has a parent, Me.Parent.Theme should be the same as Me. 99&#37; of the time, Me is noise anyway, so drop it as in my previous example (you can ignore this one; some people disagree with me. I don't use it unless it's for disambiguation.)

Use appropriate casing.
Most VB .NET developers use PascalCase for methods, properties, class names, and constants only. Parameters, local variables, fields, and other low-importance entities are camelCased. I got confused in ButtonControlTheme.OnPaintButton() because I didn't notice "Graphic" and "ReverBoarderThickness" were local variables; I thought you were somehow storing them as properties I hadn't spotted yet.

Spell words properly.
No explanation needed. Names are the most important thing in your code. I can't tell what a "ReverBoarder" is. Sounds like a danger to innkeepers. A more appropriate name would have been "BadIdea"; you're using it so you can update an important property then revert it. That's silly.

Use disposable objects wisely.
This is a bad idea from ButtonControlSkin:

Code:

---

Public ReadOnly Property Font As Font
    Get
        Return New Font("Microsoft Sans Serif", 8.25F, FontStyle.Regular)
    End Get
End Property

---

The Font class represents a GDI font. These are limited resources, so Font is a disposable resource. Unfortunately, nothing that uses this property disposes of it when finished. So every time you draw, you create a new one. You'd better hope the garbage collector runs before you run out of available font handles!

There's another problem: creating objects like fonts is slow since they're unmanaged GDI handles. ButtonControlTheme.OnPaintButton() creates 2 every time it runs since it accesses the property twice. You *could* be disposing of those fonts, but ideally you don't want to have to create them over and over. You can solve this problem by making the property a lazy property:

Code:

---

Private _font As Font
Public ReadOnly Property Font As Font
    Get
        If _font Is Nothing
            Return New Font(...)
        End If
       
        Return _font
    End Get
End Property

---

This pattern avoids creating the font if it's never used but ensures only one font is created (at least in a single-threaded situation.) It's still a GDI handle you'll have to dispose, but since you never swap out themes that's not a big deal. If you wanted to swap themes at runtime, then ButtonControlTheme should implement IDisposable and dispose of its font.

Don't write your own designer code.
It looks like you copy/pasted designer.vb files into many of your classes. There is never a good reason to do that. Don't do it.

That's all I've got so far. Typing that up has motivated me a little more. Still, of the 3,000 lines I think only about 200 are devoted to actual logic; the fun part is fixing logic. It's going to be hard to stay motivated while copy/pasting about 100 properties to where they belong. Worse, since there's no mouse interaction logic I can't check if I've mixed up some things. I think if I do anything at all it's going to be a ground-up rewrite rather than an attempt to evolve this control; that seems a heck of a lot more fun. I might even do a parallel version in WPF that has animation and other snazzy features (I need the practice, and in particular I think the day selection bar would be surprisingly easy.)

To that end, I could move a lot faster if you'd explain what the control does. I can figure most of it out, but the "schedule grid" part is mystifying without any test data. What's that supposed to look like?

Quote:

---

Originally Posted by Sitten Spynne

Spell words properly.
No explanation needed. Names are the most important thing in your code. I can't tell what a "ReverBoarder" is. Sounds like a danger to innkeepers. A more appropriate name would have been "BadIdea"; you're using it so you can update an important property then revert it. That's silly.

---

To be quite honest I have no idea what that is either, or where in fact your seeing this in my code. I'm currently at work right now and do not have my project with me, however when I get home I'll do a quick find to see where your seeing this.

Alright Sitten Spynne, I've searched up and down and saw nowhere of this "ReverBoarder". I'm thinking maybe I've removed this since I first uploaded my project here and perhaps that it could maybe be "RevertBoarder" rather "ReverBoarder". To be quite honest I don't even remember using either of the two, or of what purpose it would have been for.

As for the logic. Take a look at all my nested controls such as ButtonControl and CalendarControl. Really these can be great examples of showing you my style or "methods" of coding knowledge. I'm extremely consistent with my work I do. So these can be great examples of what I would further produce. As for all the interactions? I'm not quite there yet. Actually today I just completed my first user interactions, but whats keeping me is this database source you keep mentioning. It's exactly what you think, however I'm still in the works of designing a database, then all the functions needed to handle said database. Because of my lack of planning I feel I work backwards but still accomplish my end result. I can not complete the ScheduleGridBody Interface until I'm able to read this database. This I will contain most of my logic as well. So right now while I wait for your next post I'm designing the database, then coding the functions needed to read this database and handle it accordingly, next I'll code whats needed to display all this information, and finally the interaction between the rest of my control and the user. If I finish all this before you(<i>which I doubt</i>) then I'll upload it here.

Quote:

---

Originally Posted by DavesChillaxin

... perhaps that it could maybe be "RevertBoarder" rather "ReverBoarder". To be quite honest I don't even remember using either of the two, or of what purpose it would have been for.

....

---

That's the point.

If it was spelled correctly with a meaningful name, there would be little confusion as to what the purpose may have been.