pls review my (simple!) vba codes

[StephanV]

Because I just started to use VBA last friday, and I want to learn it more efficiently I would like to ask if you want to review my VBA code.

To keep it clear what it should do, I will give some details about my excel sheets. If you just want to see the code, please scroll down

Sheet1 = Data entry
Sheet 2 = Database

Database
The main thing is my database. This is the most important sheet of my whole workbook.
I keep a record of certificates and I need them to update every 6 months. After 2 years, the certificates receive a 'major update' and the process restarts.
To keep me informed about when a certificate has to be updated, and which certificates are allready updated I build checkboxes with VBA. This checkbox has 3 functions:
1) It keeps me informed in what 'fase' the certificate is. So 2 checks are 1 year.
2) It adds a certain amount of days to the startdate of the certificate, so that excel can give me a signal when an update is needed.
3) When checked, certain information is copied to sheet 1, where information is updated (information about the certificate)

After the two years, and 4 filled checkboxes, a commandbutton is pressed to uncheck al the checkboxes, and the process is restarted.

So every row of information has four checkboxes, and 1 commandbutton. The only problem with this process is that it requires 750 buttons, when i have 150 rows of information.

In row 1, this is the code:

Checkbox 1

Private Sub Checkbox1_Change()
If CheckBox1.Value = True Then
                    Datum = Worksheets("Sheet2").Range("H4").Value
                    Worksheets("Sheet2").Range("H4").Value = Datum + 150
                    Datum = Worksheets("Sheet2").Range("I4").Value
                    Worksheets("Sheet2").Range("I4").Value = Datum + 183
                        Dim CopyRange As Range, NextCell As Range
                            Set CopyRange = Sheet2.Range("A4,B4,E4")
                                Set NextCell = Sheet1.Range("B65536").End(xlUp).Offset(1, 0)
                            CopyRange.Copy
                            NextCell.PasteSpecial (xlValues)
                            Application.CutCopyMode = False
                            Sheets("Sheet1").Select
                        If CheckBox1.Value = False Then
                    Else
                Exit Sub
            End If
        End If
End Sub

Checkbox 2, 3 and 4 (small changes in code)

Private Sub Checkbox2_Change()
   If CheckBox1.Value = False Then
      Exit Sub
   Else
      If CheckBox2.Value = True Then
                    Datum = Worksheets("Sheet2").Range("H4").Value
                    Worksheets("Sheet2").Range("H4").Value = Datum + 150
                    Datum = Worksheets("Sheet2").Range("I4").Value
                    Worksheets("Sheet2").Range("I4").Value = Datum + 183
Dim CopyRange As Range, NextCell As Range
                            Set CopyRange = Sheet2.Range("A4,B4,E4")
                                Set NextCell = Sheet1.Range("B65536").End(xlUp).Offset(1, 0)
                            CopyRange.Copy
                            NextCell.PasteSpecial (xlValues)
                            Application.CutCopyMode = False
                            Sheets("Sheet1").Select

      Else
         Exit Sub
      End If
   End If
End Sub

Commandbutton:

Private Sub CommandButton1_click()
If CommandButton1.Enabled = True Then
        CheckBox1.Value = False
        CheckBox2.Value = False
        CheckBox3.Value = False
        CheckBox4.Value = False
    Else
        If CommandButton1.Enabled = False Then
            Exit Sub
        End If
    End If
End Sub

Data entry
Here I want two things.
1) New data entry and write thit to the last line in the database:

Private Sub CommandButton1_click()
    Call Wegschrijven_data
End Sub
 
Private Sub Wegschrijven_data()
     
    Dim CopyRange As Range, NextCell As Range
     
    Set CopyRange = Sheet1.Range("B7:G7")
    Set NextCell = Sheet2.Cells(Cells.Rows.Count, 1).End(xlUp).Offset(1, 0)
    CopyRange.Copy
    NextCell.PasteSpecial (xlValues)
    Application.CutCopyMode = False
    Worksheets("Sheet1").Range("B7:G7").Select
    Selection.ClearContents
     
End Sub

and 2) Modify certificate information and write this to a second (now it is sheet 3, this can be changed) database.

Private Sub CommandButton3_click()
    Call Wegschrijven_data2
End Sub
 
Private Sub Wegschrijven_data2()
     
    Dim CopyRange As Range, NextCell As Range
     
    Set CopyRange = Sheet1.Range("B12:F" & Range("B" & Rows.Count).End(xlUp).Offset(1, 0).Row)
    Set NextCell = Sheet3.Cells(Cells.Rows.Count, 1).End(xlUp).Offset(1, 0)
    CopyRange.Copy
    NextCell.PasteSpecial (xlValues)
    Application.CutCopyMode = False
    Worksheets("Sheet1").Range("B12:F" & Range("B" & Rows.Count).End(xlUp).Offset(1, 0).Row).ClearContents
    Selection.ClearContents
     
End Sub

I really hope you would give me some comments on my vba codes! I know it is a lot of information, but without it, it makes no sense.

 

[cornflakegirl]

Stephan

Four checkboxes and one button per line sounds very busy. Are you sure this is the best way to do it? Maybe you could post a screenshot of your database sheet - we might be able to suggest some alternative methods.

On your actual question - I've only looked at your first bit of code, but here are some comments on it:

Private Sub Checkbox1_Change()
 
Dim CopyRange As Range, NextCell As Range 'Dim variables at the start - keeps it neat, and it's where people expect to see it
'Also, it's good practice to put Option Explicit at the start of your code - forces you to Dim everything.
'That way, if you misspell a variable, Excel will object, rather than your code just not working.
 
If CheckBox1.Value = True Then 'Your nesting is odd. The point of nesting is to make the code easier to follow, but yours didn't really do this.
     Worksheets("Sheet2").Range("H4").Value = Worksheets("Sheet2").Range("H4").Value + 150 'don't really need Datum
     Worksheets("Sheet2").Range("I4").Value = Worksheets ("Sheet2").Range("I4").Value + 183                   
     Set CopyRange = Sheet2.Range("A4,B4,E4") 'again, you don't really need the variable, as you're only using it once - but it doesn't hurt
     Set NextCell = Sheet1.Range("B65536").End(xlUp).Offset(1, 0)
     CopyRange.Copy                       
     NextCell.PasteSpecial (xlValues)                           
     Application.CutCopyMode = False                       
     Sheets("Sheet1").Select
End If                      
' you don't need to check if CheckBox1.Value is False - that's the only other option
' and if your Else is "do nothing", you can just omit it                      
End Sub

 

[StephanV]

Hello Emma,

First, thank you for looking at my code!

Because everybody suggested that its not a good idea to use that many checkboxes I erased 4 of the 5 boxes per row. Now I still have 1 checkbox per row. And I have 250 rows.

This is the code:

Private Sub CommandButton1_click()
    Call Wegschrijven_data1
End Sub
 
Private Sub Wegschrijven_data1()
         Dim CopyRange As Range, NextCell As Range
        Set CopyRange = Sheet2.Range("A4,B4,E4,L4,M4,N4,O4")
        Set NextCell = Sheet3.Cells(Cells.Rows.Count, 1).End(xlUp).Offset(1, 0)
    CopyRange.Copy
    NextCell.PasteSpecial
    Application.CutCopyMode = False
           Sheets("Sheet2").Select
        Range("L4:O4").Select
        Application.CutCopyMode = False
        Selection.ClearContents
        Datum = Worksheets("Sheet2").Range("F4").Value
        Worksheets("Sheet2").Range("F4").Value = Datum + 730
End Sub

Because im just a beginner at VBA, my idea was to make 250 of these checkboxes (generate them true PHP), but this idea was burned by users on an other forum .

First, is this code so bad?
Second, how can I make 250 of these boxes...using parameters? How does this work?