2
\$\begingroup\$

I am going to rewrite my AddIn I made for the Autodesk Inventor software. It is supposed to do specific task that are not available by default in the program.

I am not a programmer but a draftsman that is interested in programming, so bare with me.

The reason for me to rewrite my program is to make it better documented and commented. Plus there is alot of code that is duplicated and hardcoded lines that must be removed.

Now I made a small sample for a class that I am doing, and how I think it should be commented and xml documented. Please feel free to comment on ALL the code not only the xml tags.

Imports Inventor

''' <summary>
''' The document object represents the Inventor Document, 
''' it allows the you to rapidly access certain values and perform specific tasks
''' </summary>
Public Class Document

    ''' <summary>
    ''' Read-only property that returns the <c>Inventor.Document</c> that is associative with the XT document.
    ''' </summary>
    ''' <example>Document.ReferencedDocument() As Inventor.Document</example> 
    Public Overridable ReadOnly Property ReferencedDocument As Inventor.Document


    ''' <summary>
    ''' Creates a new XT document object
    ''' </summary>
    ''' <param name="InventorDocument">The Inventor document where the new XT document will be associated with.</param>
    ''' <example>Dim XTdocument as New Document(InventorDocument As Inventor.Document)</example> 
    Public Sub New(ByVal InventorDocument As Inventor.Document)

        ' Store the InventorDocument as ReferencedDocument
        ReferencedDocument = InventorDocument
    End Sub

    ''' <summary>
    ''' Gets the description of the current document
    ''' </summary>
    ''' <returns>string</returns>
    Public Property Description As String
        Get

            ' Set reference to the document property sets
            Dim oPropertySets As PropertySets =
                ReferencedDocument.PropertySets

            ' Set reference to the Design Tracking Property set
            Dim oDesignTrackingPropertySet As PropertySet =
                oPropertySets.Item("Design Tracking Properties")

            ' Set reference to the Description property
            Dim oDescriptionProperty As [Property] =
                oDesignTrackingPropertySet.Item("Description")

            ' Return the description value (as string)
            Description = oDescriptionProperty.Value

        End Get
        Set(value As String)

            ' Set reference to the document property sets
            Dim oPropertySets As PropertySets =
                ReferencedDocument.PropertySets

            ' Set reference to the Design Tracking Property set
            Dim oDesignTrackingPropertySet As PropertySet =
                oPropertySets.Item("Design Tracking Properties")

            ' Set reference to the Description property
            Dim oDescriptionProperty As [Property] =
                oDesignTrackingPropertySet.Item("Description")

            ' Return the description value (as string)
            oDescriptionProperty.Value = value

        End Set
    End Property



End Class
\$\endgroup\$
1
\$\begingroup\$

Only focusing on the code...no, not really, I will say something regarding the xml doc as well.

  • xml documentation should tell the truth, because this is what another developer reads. Seeing e.g ''' Gets the description of the current document one wouldn't expect that this property has a setter as well.
    ''' Creates a new XT document object isn't true either, because it is used to create a new Document object. If you have another class named XTDocument inheriting from the Document object then the doc would be ok.

  • Although VB.NET is case insensitive one should stick to the .NET Naming Guidelines. This means method parameters should be named using camelCase casing.

  • While we are at naming things, please don't use hungarian notation. Prefixing a variable with its type or with what one thinks is its type is frowned upon nowadays. What value does the o in oPropertySets add ? Having intellisense and stuff just makes it superflous. In the time of VB 6 one could say that it is easier to see the type of the variable, but nowadays one just hoovers with his mouse over the variables name and see its type.

  • Comments shouldn't tell what the code is doing. This should be done by the code itself by using meaningful names. Comments should always tell why the code is written like it is. As an example, please take a look at my answer here.

  • Nowadays (sorry I need to use this term often, because the code looks like it is ported from VB 6) we use Return someValue instead of assigning the returning values to the properties name.
    Not only because it is more clear but in VB.NET this Description = oDescriptionProperty.Value will call the setter of the Description property. Hence calling the getter will call the setter as well which can lead to dangerous, because hidden, sideeffects.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the review, I appreciate it alot! let me quickly comment on those points. * \$\endgroup\$ – Mech_Engineer Jan 24 '18 at 7:24
  • \$\begingroup\$ 1) Correct it would be better to write Get/Sets the.... 2) I will read into it, good reference link! 3) I created this habit from the Autodesk Inventor API sample, here it is used frequently oProperty. I feel it makes my AddIn code more readable when using the API documentation from Inventor, might not follow-up this advise. 4) I am asked to comment every step of code and what it does (no real programmers here, so if I would quite nobody understands anything of it) 5) You are correct. It is my first for expanded getter and setter properties good advise! \$\endgroup\$ – Mech_Engineer Jan 24 '18 at 7:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.