Coding style: What do you think is better?

[JelteDeJong]

I have 4 rules that do exactly the same thing. Just out of curiosity I would like to know what you think is the best code snippet? Should the code be as short as possible and what about readability?

Snippet 1:

Dim view As DrawingView = ThisApplication.CommandManager.Pick(SelectionFilterEnum.kDrawingViewFilter, "Select a drawing view")
If (view Is Nothing) Then Return
view.DrawingCurves.Cast(Of DrawingCurve).Where(Function(c) c.EdgeType = DrawingEdgeTypeEnum.kBendDownEdge Or c.EdgeType = DrawingEdgeTypeEnum.kBendUpEdge).ToList().ForEach(Function(c) view.Parent.DrawingNotes.BendNotes.Add(c))

Snippet 2:

Dim view As DrawingView = ThisApplication.CommandManager.Pick(
    SelectionFilterEnum.kDrawingViewFilter,
    "Select a drawing view")

If (view Is Nothing) Then Return

view.DrawingCurves.Cast(Of DrawingCurve).
    Where(Function(c) c.EdgeType = DrawingEdgeTypeEnum.kBendDownEdge Or
                      c.EdgeType = DrawingEdgeTypeEnum.kBendUpEdge).ToList().
    ForEach(Function(c) view.Parent.DrawingNotes.BendNotes.Add(c))

 Snippet 3:

Dim view As DrawingView = ThisApplication.CommandManager.Pick(
	           SelectionFilterEnum.kDrawingViewFilter,
	           "Select a drawing view")
If (view Is Nothing) Then Return
	
Dim bendCurves = view.DrawingCurves.Cast(Of DrawingCurve).
    Where(Function(curve) curve.EdgeType = DrawingEdgeTypeEnum.kBendDownEdge Or
                          curve.EdgeType = DrawingEdgeTypeEnum.kBendUpEdge).ToList()
						  
Dim bendNotes = view.Parent.DrawingNotes.BendNotes
For Each curve As DrawingCurve In bendCurves
    bendNotes.Add(curve)
Next

 Snippet 4:

' Let the user select the view that he/she wants to update.
Dim commandManager As CommandManager = ThisApplication.CommandManager
Dim view As DrawingView = commandManager.Pick(
       SelectionFilterEnum.kDrawingViewFilter,
       "Select a drawing view")

' Check if the user selected a view.
If (view Is Nothing) Then
    ' If not stop the rule.
    Return
End If

' Create a list of all bend cures
Dim bendCUrves As List(Of DrawingCurve)
bendCUrves = New List(Of DrawingCurve)
For Each curve As DrawingCurve In view.DrawingCurves
    If (curve.EdgeType = DrawingEdgeTypeEnum.kBendDownEdge) Then
        bendCUrves.Add(curve)
    End If
    If (curve.EdgeType = DrawingEdgeTypeEnum.kBendUpEdge) Then
        bendCUrves.Add(curve)
    End If
Next

' Add bend notes to all sheets.
Dim sheet As Sheet = view.Parent
Dim bendNotes = sheet.DrawingNotes.BendNotes
For Each curve As DrawingCurve In bendCUrves
    bendNotes.Add(curve)
Next

[JelteDeJong]

By the way, this rule will generate all bend notes. More info on my blog post "Automatically generate bend notes"

[florian_wenzel]

Hi,

i prefer Snippet 4, because has Comment.

And this is very good Thing, for yourself, and for other 😉

[JelteDeJong]

I agree that comments can be very useful. But if you ask me the code should make itself clear. Consider snippet 1 and add comments. Something like this.

' Let the user select the view that he/she wants to update.
Dim view As DrawingView = ThisApplication.CommandManager.Pick(SelectionFilterEnum.kDrawingViewFilter, "Select a drawing view")
' Check if the user selected a view. If not stop the rule.
If (view Is Nothing) Then Return
' Create a list of all bend cures and add bend notes to all sheets.
view.DrawingCurves.Cast(Of DrawingCurve).Where(Function(c) c.EdgeType = DrawingEdgeTypeEnum.kBendDownEdge Or c.EdgeType = DrawingEdgeTypeEnum.kBendUpEdge).ToList().ForEach(Function(c) view.Parent.DrawingNotes.BendNotes.Add(c))

 Even with comments, it's hard to read.

[Michael.Navara]

All of them are useful and readable for me. Which of them is the best, depends on reason and who is the consumer of this snippet.

I prefer to split collecting the objects and do some action on them. Because often I need more complex action then one line of code (error handling, logging, comments, etc.) (=> Snippets 3, 4)

I don't know how familiar with LINQ are members of this forum. If they want to modify or reuse the snippet, it can be little bit confusing for them. (=> Snippet 4)

Snippet 1 is very compressed form and it is suitable for some internal functions in addins or other "closed" code.

Snippet 2 is nice example how to use LINQ for simple task, but as I mentioned above, it can be hard to modify this code for many of non-advanced users.

[vpeuvion]

Hi,

If the code is intended to be modified or transferred to different people, for me snippet 4 is the best with or without comments.

The other excerpts are more difficult to read, especially for someone who didn't write it, and when you come back to work on it 6 months later, it's also more difficult to remember what you had chosen earlier.

It's just my personal opinion.

Vincent.

[basautomationservices]

For me its a close call between 2 and 3. Concise and clear what happens. Code explains itself so no comments are required imo.

Since you're using LINQ already you might as well go all the way. So snippet 2 will win.

While being the same as snippet 1 it's alot more readable due to the formatting. If not using iLogic I would create a method for checking the edgetype and the adding of the bendnote to add some logging / error handling. 

I agree with Michael Navara that for non-advanced users snippet 4 wins. For me it's just too much unnecessary typing 🙂

[Zach.Stauffer]

I'd go with Snippet 3, because I personally prefer a more verbose style than something like Snippet 2. I like to have each "task" have it's own section in code. The main benefit is when I come back to it 6 months later, it's easier for me to tell what the code does.

Snippet 4 is good for beginners but as a general rule code should be understandable without comments, and I think #2 and #3 are fairly easy to understand. If I do put in comments I tend to put them at the end of a line of code, not before a block. That way I can avoid them if I don't need them, but they are there if I do.

[JelteDeJong]

Personally, I don’t think snippet 1 is good in any situation. It is too compressed to read. My personal guild line is that a line of code should never be longer than 100 characters and a function should never be longer than what fits on the screen. (However, I find myself often in situations where functions are too long.) That is also why I think snippet 2 is better than 1.

But Using the foreach function from LINQ is not something I do often. Most of the time I have multiple lines of code in a foreach loop and I don’t think you should use multiple lines of code in a LINQ function.  

So I think snippet 3 is the best if I expect that my code will only be read by someone with some coding experience. But I agree with @Michael.Navara that on this forum we often need to explain a little bit more. And maybe then snippet 4 is then the best. But then again I agree with wit @basautomationservices for me it's just too much unnecessary typing. And I don’t think that LINQ code is that hard to read.

Anyway, I guess we are all more on less on the same page. Thank you for your comments.

[tomislav.peran]

I vote for 3 or 4. 

4 would be the best because it has an explanation. That helps the user to understand a bit about what is going on. 

[_dscholtes_]

I would choose for snippet 4. Writing code is not my profession, so I would not compact it. I use blank lines to separate associated lines of code (dim & set combo's) and use (short) comments as much as I can. 
I would however create a single line if-statement for the check if the user selected a view.
I would create an if-else-statement instead of the two if-end if statements for the curves.

[pball]

I'd likely be between 2 and 3. I'm familiar with Linq now and enjoy using it but I'm by no means proficient with it. So snippet 3 is more likely something I'd normally write. I've been trying to add more comments to my code in general, mostly to help myself in the future since I'm the only one coding for Inventor around my place.

[WCrihfield]

I like the first part of Snippet 1, and the last half of Snippet 3 about the best.  The Pick method can just be a 1 liner, because it's not overly long.  The check for Nothing can also just be a 1 liner right after that, instead of a 3 line block.  But I would prefer to split that really long last part of Snippet 1 into at least 2-3 parts as is done in Snippet 3.  Comments are OK if the code is complicated to wrap your mind around or hard to tell what's going on if you were to read again a year later, or someone else may be inheriting your code after you have moved on.  I like a good marriage between somewhat condensed, and yet still easy enough for some random mid-level, part time programmer to be able to follow what's going on later.