Indenting and Spacing Revisited. (3 Viewers)

[Thales750]

If you have a moment, please take a look at this and tell me how you would indent it. I'm forming a whole new standard, thougt it was perfect time to entertain some new ideas.
Happy New Year Y'all.

Private Sub Form_Click()
    Me.txtHighlight.Requery
 

Dim Rs As DAO.Recordset
Dim ctl As Control
Dim varItem As Variant
  
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
      
            Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
        Call SelectAll(Forms!frmProjectManagement.lstFileType)
  
            strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
                "FROM tblLocalFilter;"
                CurrentDb.Execute strSQL, dbFailOnError
  
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
  
            If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
                Me.Parent.lstFileType.Requery
                Me.Parent.frmActionItemAttachmentsSubform.Requery
            Exit Sub
            End If
  
    Set ctl = Me.Parent.lstFileType
        For Each varItem In ctl.ItemsSelected
            Rs.AddNew
            Rs!lfFileTypeID = ctl.ItemData(varItem)
              
Debug.Print ctl.ItemData(varItem)
              
            Rs.Update
            Next varItem
    Set Rs = Nothing
  
            Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

 

[plog]

Private Sub Form_Click()
   ' Comment summarizing what this sub does
 
Dim Rs As DAO.Recordset      ' comment about what this will do
Dim ctl As Control                  ' comment about what this will do
Dim varItem As Variant           ' comment about what this will do

pubProjectID = Me.txtSourceID
pubEntityID = pubProjectID
pubEntityTableID = 3

' dim declaration and initialization before any actual coding above


Me.txtHighlight.Requery
Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
Call SelectAll(Forms!frmProjectManagement.lstFileType)
strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
    "FROM tblLocalFilter;"

CurrentDb.Execute strSQL, dbFailOnError
Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")

If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
  ' comment about what this IF is doing
    Me.Parent.lstFileType.Requery
    Me.Parent.frmActionItemAttachmentsSubform.Requery
    Exit Sub
    End If
 
Set ctl = Me.Parent.lstFileType

For Each varItem In ctl.ItemsSelected
  ' comment about what this foreach is doing
    Rs.AddNew
    Rs!lfFileTypeID = ctl.ItemData(varItem)
            
    Debug.Print ctl.ItemData(varItem)
   
    Rs.Update
    Next varItem

Set Rs = Nothing
Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

Things should get indented when you are going into a controlled block (IF/Switch/Foreach/While, etc.)

 

[pbaldy]

MZ Tools indents it like this, which I'm okay with.

Private Sub Form_Click()
  Me.txtHighlight.Requery
 

  Dim Rs As DAO.Recordset
  Dim ctl As Control
  Dim varItem As Variant
 
  pubProjectID = Me.txtSourceID
  pubEntityID = pubProjectID
  pubEntityTableID = 3
      
  Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
  Call SelectAll(Forms!frmProjectManagement.lstFileType)
 
  strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
    "FROM tblLocalFilter;"
  CurrentDb.Execute strSQL, dbFailOnError
 
  Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
 
  If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
    Me.Parent.lstFileType.Requery
    Me.Parent.frmActionItemAttachmentsSubform.Requery
    Exit Sub
  End If
 
  Set ctl = Me.Parent.lstFileType
  For Each varItem In ctl.ItemsSelected
    Rs.AddNew
    Rs!lfFileTypeID = ctl.ItemData(varItem)
              
    Debug.Print ctl.ItemData(varItem)
              
    Rs.Update
  Next varItem
  Set Rs = Nothing
 
  Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

I preferred how Smart Indenter handled SQL statements. Instead of the above it would do:

  strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
           "FROM tblLocalFilter;"

 

[The_Doc_Man]

I might have chosen other indentation schemes. However, my opinion on your scheme is simple: If you can maintain it consistently and if it makes sense to you regarding WHY you pick a particular level of indentation - including what it means to be indented x, y, or z spaces, then it is as good as any other scheme. Because the first two rules of ANY style of programming are (1) Do you understand it? and (2) Can you continue its use? By the way, this is very similar to ANOTHER common question here - variable naming conventions. Will you use it? Does it help you? Then I'm all for you continuing to use it.

 

[DickyP]

.... By the way, this is very similar to ANOTHER common question here - variable naming conventions. Will you use it? Does it help you? Then I'm all for you continuing to use it.

Have been using the Lesynski-Reddick naming conventions since the 1990s and would recommend them to users - as Doc mans says - if it works!

Lesynski-Redick naming convention.

 

[GPGeorge]

Consistency and usability. The two most important factors.

 

[Thales750]

I might have chosen other indentation schemes. However, my opinion on your scheme is simple: If you can maintain it consistently and if it makes sense to you regarding WHY you pick a particular level of indentation - including what it means to be indented x, y, or z spaces, then it is as good as any other scheme. Because the first two rules of ANY style of programming are (1) Do you understand it? and (2) Can you continue its use? By the way, this is very similar to ANOTHER common question here - variable naming conventions. Will you use it? Does it help you? Then I'm all for you continuing to use it.

The why is because I can't read list. I absolutely love outlines.if you notice, I have more I indentions. The pattern of tabulstions facilitates at a glance recon.

What about variables?

 

[MajP]

If that formatting works for you then have at it. Do whatever works for you.
However, I would imagine most people reading your code would hate it. To me that is a mess and hard to read. For most of us this is a bad solution for a non-problem.

I want to clearly see the code blocks, this is the most import reason to do any editing. If they are lined up that makes sense.

Start of code block
   some code
   start of other code block
       some code
    end of other code block
end of Code block

This is like helter skelter to me

et ctl = Me.Parent.lstFileType
        For Each varItem In ctl.ItemsSelected
            Rs.AddNew
            Rs!lfFileTypeID = ctl.ItemData(varItem)
              
Debug.Print ctl.ItemData(varItem)
              
            Rs.Update
            Next varItem
    Set Rs = Nothing

Again, you might have the magic decoder ring to make sense out of this, but for me this is painful to read.

 

[The_Doc_Man]

What about variables?

The comment was merely that your question about indenting and isolated statements was a thing similar to questions about how to build variable names, with prefixes like "l" for LONG and "i" for INTEGER and "d" for DOUBLE, etc. Naming conventions, like tabbing and spacing, are more along the lines of "beauty is in the eye of the beholder." No more and no less than that.

 

[isladogs]

Again, you might have the magic decoder ring to make sense out of this, but for me this is painful to read.

I would agree with that. In addition, if your code is ever to be viewed / edited by other developers, then using a 'standard approach' to indentation will ensure it is easier for others to read/navigate

 

[Thales750]

If that formatting works for you then have at it. Do whatever works for you.
However, I would imagine most people reading your code would hate it. To me that is a mess and hard to read. For most of us this is a bad solution for a non-problem.

I want to clearly see the code blocks, this is the most import reason to do any editing. If they are lined up that makes sense.

Start of code block
   some code
   start of other code block
       some code
    end of other code block
end of Code block

This is like helter skelter to me

et ctl = Me.Parent.lstFileType
        For Each varItem In ctl.ItemsSelected
            Rs.AddNew
            Rs!lfFileTypeID = ctl.ItemData(varItem)
             
Debug.Print ctl.ItemData(varItem)
             
            Rs.Update
            Next varItem
    Set Rs = Nothing

Again, you might have the magic decoder ring to make sense out of this, but for me this is painful to read.

I put debug all the way to the left so I can find them later and delete them.

 

[Pat Hartman]

I put debug all the way to the left so I can find them later and delete them.

And I simply search for "debug". It's a way

 

[MajP]

I put debug all the way to the left so I can find them later and delete them

I get that but it is the non-aligned code blocks I have a hard time with.

       For Each varItem In ctl.ItemsSelected
            ....
            Next varItem

 

[Isaac]

If you have a moment, please take a look at this and tell me how you would indent it. I'm forming a whole new standard, thougt it was perfect time to entertain some new ideas.
Happy New Year Y'all.

Private Sub Form_Click()
    Me.txtHighlight.Requery
 

Dim Rs As DAO.Recordset
Dim ctl As Control
Dim varItem As Variant
 
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
     
            Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
        Call SelectAll(Forms!frmProjectManagement.lstFileType)
 
            strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
                "FROM tblLocalFilter;"
                CurrentDb.Execute strSQL, dbFailOnError
 
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
 
            If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
                Me.Parent.lstFileType.Requery
                Me.Parent.frmActionItemAttachmentsSubform.Requery
            Exit Sub
            End If
 
    Set ctl = Me.Parent.lstFileType
        For Each varItem In ctl.ItemsSelected
            Rs.AddNew
            Rs!lfFileTypeID = ctl.ItemData(varItem)
             
Debug.Print ctl.ItemData(varItem)
             
            Rs.Update
            Next varItem
    Set Rs = Nothing
 
            Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

My personal indentation mental machine , can offer a few comments without showing how I would but simply calling out specific differences.
Half the indentation here is unnecessary, such as:

• The indentation of the entire procedure inside the Private Sub / End Sub. (this is one more Tab to keep typing, yields no benefit).
• The random indents on lines like If Me.Parent.1stfileType, as well as For each varItem in ctl.ItemsSelected.

I guess the most succinct way I could say this is that at least half the indentations are totally uncalled for. Indentations should show belonging and hierarchy. "for each varitem" does not belong to "set ctl = me.parent.1stfiletype", they are 2 unrelated things at the same level.

 

[Isaac]

Consistency and usability. The two most important factors.

Good ones! And what does the indent TELL you? The indented thing should sort of 'belong' to/inside the outdented thing above it.

It makes no sense to suddenly begin an indent when there is no change of level of hierarchy, makes it look like something belongs to something that it doesn't.

 

[Isaac]

If that formatting works for you then have at it. Do whatever works for you.
However, I would imagine most people reading your code would hate it.

Agree with your last statement.
As for the first statement, well my rule is, "if the formatting works for the greatest number of people who will come AFTER you, Do it".

Anyone can get used to reading their own code, this is an illegitimate release from the duty of formatting.
The question is who else can read your code. If you're the only one who understands it, you're amiss.

Consistency is high on the priority list, but consistently doing something that makes no sense to the average person who comes after you is no good. I used to know a guy who literally wrote sql all running together on the same line(s). It made perfect sense to him, but people who had to read his code wanted to shoot themselves. I'm very passionate about good code formatting. For example, people ask how to format Joins in t-sql. My rule is simple - every line of code that produces a new Relation gets its own indent and line.
i.e.

select
    col1,
    col2
from
    table1
    inner join table2 on condition
    inner join table3 on condition

 

[Josef P.]

If you are thinking about the spacing, this could also be a sign that the procedures have too much different content.

 

[Edgar_]

Without modifying the code, this is how I would approach indenting and spacing:

Private Sub Form_Click()

    Me.txtHighlight.Requery
  
    Dim Rs As DAO.Recordset
    Dim ctl As Control
    Dim varItem As Variant
  
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
  
    Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
  
    Call SelectAll(Forms!frmProjectManagement.lstFileType)
  
    strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
    "FROM tblLocalFilter;"
  
    CurrentDb.Execute strSQL, dbFailOnError
  
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
  
    If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
        Me.Parent.lstFileType.Requery
        Me.Parent.frmActionItemAttachmentsSubform.Requery
        Exit Sub
    End If
  
    Set ctl = Me.Parent.lstFileType
    For Each varItem In ctl.ItemsSelected
        Rs.AddNew
        Rs!lfFileTypeID = ctl.ItemData(varItem)
      
        Debug.Print ctl.ItemData(varItem)
      
        Rs.Update
    Next varItem
  
    Set Rs = Nothing
  
    Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

However, I notice it has too many line breaks, as I’ve been trying to separate ideas within this routine. To keep the same functionality while improving the readability for myself, I would rearrange a few lines and format it like this:

Private Sub Form_Click()

    ' do this first
    Me.txtHighlight.Requery
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
   
    ' now delete this
    CurrentDb.Execute "DELETE tblLocalFilter.rfReportFilterID FROM tblLocalFilter;", dbFailOnError
   
    ' then do this with lstFileType
    Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
    Call SelectAll(Forms!frmProjectManagement.lstFileType)
   
    With Me.Parent
        If .lstFileType.ItemsSelected.Count = 0 Then
            .lstFileType.Requery
            .frmActionItemAttachmentsSubform.Requery
            Exit Sub
        End If
    End With
   
    ' now take care of this recordset
    Dim Rs As DAO.Recordset
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
   
    Dim varItem As Variant
    Dim ctl As Control
    Set ctl = Me.Parent.lstFileType
    For Each varItem In ctl.ItemsSelected
        Rs.AddNew
        Rs!lfFileTypeID = ctl.ItemData(varItem)
       
        Debug.Print ctl.ItemData(varItem)
       
        Rs.Update
    Next varItem
   
    ' finally do this
    Set Rs = Nothing
    Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

I did this:
1. Moved variable declarations one line before they are used
2. Made an initial block of things that should happen in the beginning
3. Modified the SQL instruction because it does not change programmatically and it's short enough
4. Put a WITH where the dot notation was getting too repetitive
5. Declared variables in order of appearance before the loop
6. Made a final block of things that should happen in the end

And it's easier for me to read now. A matter of preference, but this function is doing too many things.

 

[Isaac]

Without modifying the code, this is how I would approach indenting and spacing:

Private Sub Form_Click()

    Me.txtHighlight.Requery
  
    Dim Rs As DAO.Recordset
    Dim ctl As Control
    Dim varItem As Variant
  
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
  
    Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
  
    Call SelectAll(Forms!frmProjectManagement.lstFileType)
  
    strSQL = "DELETE tblLocalFilter.rfReportFilterID " & vbCrLf & _
    "FROM tblLocalFilter;"
  
    CurrentDb.Execute strSQL, dbFailOnError
  
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
  
    If Me.Parent.lstFileType.ItemsSelected.Count = 0 Then
        Me.Parent.lstFileType.Requery
        Me.Parent.frmActionItemAttachmentsSubform.Requery
        Exit Sub
    End If
  
    Set ctl = Me.Parent.lstFileType
    For Each varItem In ctl.ItemsSelected
        Rs.AddNew
        Rs!lfFileTypeID = ctl.ItemData(varItem)
      
        Debug.Print ctl.ItemData(varItem)
      
        Rs.Update
    Next varItem
  
    Set Rs = Nothing
  
    Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

However, I notice it has too many line breaks, as I’ve been trying to separate ideas within this routine. To keep the same functionality while improving the readability for myself, I would rearrange a few lines and format it like this:

Private Sub Form_Click()

    ' do this first
    Me.txtHighlight.Requery
    pubProjectID = Me.txtSourceID
    pubEntityID = pubProjectID
    pubEntityTableID = 3
   
    ' now delete this
    CurrentDb.Execute "DELETE tblLocalFilter.rfReportFilterID FROM tblLocalFilter;", dbFailOnError
   
    ' then do this with lstFileType
    Me.Parent.lstFileType.RowSource = "qryFileTypeGroup"
    Call SelectAll(Forms!frmProjectManagement.lstFileType)
   
    With Me.Parent
        If .lstFileType.ItemsSelected.Count = 0 Then
            .lstFileType.Requery
            .frmActionItemAttachmentsSubform.Requery
            Exit Sub
        End If
    End With
   
    ' now take care of this recordset
    Dim Rs As DAO.Recordset
    Set Rs = CurrentDb.OpenRecordset("tblLocalFilter")
   
    Dim varItem As Variant
    Dim ctl As Control
    Set ctl = Me.Parent.lstFileType
    For Each varItem In ctl.ItemsSelected
        Rs.AddNew
        Rs!lfFileTypeID = ctl.ItemData(varItem)
       
        Debug.Print ctl.ItemData(varItem)
       
        Rs.Update
    Next varItem
   
    ' finally do this
    Set Rs = Nothing
    Me.Parent.frmActionItemAttachmentsSubform.Requery

End Sub

I did this:
1. Moved variable declarations one line before they are used
2. Made an initial block of things that should happen in the beginning
3. Modified the SQL instruction because it does not change programmatically and it's short enough
4. Put a WITH where the dot notation was getting too repetitive
5. Declared variables in order of appearance before the loop
6. Made a final block of things that should happen in the end

And it's easier for me to read now. A matter of preference.

@Edgar_ I'm a bit curious on your choice of where to put variable declarations. Can you supply your thinking on the matter? It may educate and challenge my own.

 

[Edgar_]

@Edgar_ I'm a bit curious on your choice of where to put variable declarations. Can you supply your thinking on the matter? It may educate and challenge my own.

In general, I like to separate the ideas within code, so these are a few considerations:
If the variable won't be used yet, I don't want to deal with it.
If I change the code later and I don't need that variable anymore, I don't want to check whether the variable is orphaned some other place.
If I will refactor the code, I want to copy paste more easily, instead of copying one block from here and another from there.
I don't like to declare variables inside a loop, so I put them outside for the loop to reassign them and make them literal variable things, but because there might be a few, I declare them in order of appearance.

There may be other considerations I'm missing.