Author Topic: Tidying up  (Read 157 times)

0 Members and 1 Guest are viewing this topic.

pjm8765

  • Newt
  • Posts: 21
Tidying up
« on: July 13, 2017, 06:13:11 am »
Related to my previous post.

Code: [Select]

                        Dim curves As DBObjectCollection = GetNewCurves(myDocuments, slabRegion, updatedSpanLine, localSpanVector, entForExtension, jaggedLine, slabAreaPLine, bStretch, PointToTrim, bIsSlabUpdated)
                        slabAreaPLine.Delete()
                        If bIsSlabUpdated = False Then
                            Exit Sub
                        End If

                        'Create a new Region
                        CreateNewRegion(curves, slab, myDocuments, regCentroid)


The above code is buried in a for each loop of block refs, so for each new block reference it's DIMing a new "curves" variable (not my choice of coding design).  Do I need to clear out this variable before it gets reDIMed or can I leave the garbage collection to pick this up and remove it?

kdub

  • SuperMod
  • Swamp Rat
  • Posts: 947
  • class keyThumper<T>:ILazy<T>
Re: Tidying up
« Reply #1 on: July 13, 2017, 06:27:18 am »
VB does some stuff I don't pretend to understand,
but, in principle :
declare the variable outside the loop
and assign the value in the loop is the accepted and conventional way to do it ... though it may depend on what else you have going on that we don't know about.

I'm surprised the compiler lets you re-declare the variable.

Regarding the mechanics;

Are you newing up a DBObjectCollection  in GetNewCurves and passing it back to the variable curves ??
« Last Edit: July 13, 2017, 06:30:23 am by kdub »
called Kerry in my other life

Sometimes the question is more important than the answer.

pjm8765

  • Newt
  • Posts: 21
Re: Tidying up
« Reply #2 on: July 13, 2017, 07:16:35 am »
I'm afraid my predecessors had a predilection for declaring variables all over a function/procedure like this and for writing multilooped functions (usually processing entities for a selection of block references) in excess of hundreds of lines of code..see below for a good example.  However, I have learnt through bitter experience not to meddle with it if it isn't broken!

Yes, the GetNewCurves function returns an object created in that function :

Code: [Select]
    Private Function GetNewCurves(ByRef myDocuments As AcadDocumentManagerExample,
                                  ByRef slabRegion As AcadRegion, ByRef updatedSpanLine As Line, ByVal localSpanVector As String, ByRef entForExtension As AcadEntity,
                                  ByRef jaggedLine As Line, ByVal slabAreaPLine As AcadLWPolyline, ByVal bStretch As Boolean, ByVal PointToTrim As Point3d, ByRef bIsSlabUpdated As Boolean) As DBObjectCollection
        Dim SpanStartX, SpanStartY, SpanEndX, SpanEndY As Double
        Dim ptPrev As Point3d = Point3d.Origin
        Dim ptVertLinePt As Point3d = Point3d.Origin
        Dim curves As DBObjectCollection = New DBObjectCollection()
        Dim Ent As AcadEntity
        Dim lstUpdatedLines As List(Of String) = New List(Of String)
        Try
            'Get Span coords from Block attributes
            Dim args() As String = localSpanVector.Split(",")
            If args.Length = 4 Then
                SpanStartX = (Val(args(0)) * FImperialScaleFactor)
                SpanStartY = (Val(args(1)) * FImperialScaleFactor)
                SpanEndX = (Val(args(2)) * FImperialScaleFactor)
                SpanEndY = (Val(args(3)) * FImperialScaleFactor)
            End If

            'Get exploded lines of the region
            If Not slabRegion Is Nothing Then
                'Get Vertical Lines
                Dim lstVerticalLines As List(Of Line) = GetVerticalLines(slabRegion, SpanStartX, SpanStartY, SpanEndX, SpanEndY, myDocuments)

                'Explode the region to extend its lines
                Dim unOrderexplodedKr As Object = slabRegion.Explode
                Dim explodedKr As Object = orderexpl(unOrderexplodedKr, myDocuments)
                For ixs As Integer = 0 To UBound(explodedKr)
                    If Not ((TypeOf (explodedKr(ixs)) Is AcadRegion) Or (TypeOf (explodedKr(ixs)) Is AcadPoint)) Then
                        If TypeOf (explodedKr(ixs)) Is AcadLine Then
                            'Get the points of the line
                            Dim line As AcadLine = explodedKr(ixs)
                            Dim ptStart As Point3d = New Point3d(line.StartPoint(0), line.StartPoint(1), 0)
                            Dim ptEnd As Point3d = New Point3d(line.EndPoint(0), line.EndPoint(1), 0)

                            'Dont extend vertical lines. Just add it in curves array.
                            Dim bIsVertical As Boolean = CheckForVerticalLine(line, lstVerticalLines)
                            If bIsVertical Then
                                If Not lstUpdatedLines.Contains(line.Handle) Then
                                    curves.Add(New Line(ptStart, ptEnd))
                                End If
                                Continue For
                            End If

                            'Check if the line is a Span
                            Dim orgSpanLine As Line2d = New Line2d(New Point2d(SpanStartX, SpanStartY),
                                                               New Point2d(SpanEndX, SpanEndY))
                            Dim bSpan As Boolean = False
                            If orgSpanLine.GetDistanceTo(New Point2d(ptStart.X, ptStart.Y)) < 1 And orgSpanLine.GetDistanceTo(New Point2d(ptEnd.X, ptEnd.Y)) < 1 Then
                                bSpan = True
                                updatedSpanLine = New Line(ptStart, ptEnd)
                            End If

                            'Check if the line is a jagged edge
                            Dim bJagged As Boolean = False
                            If Not jaggedLine Is Nothing Then
                                Dim jaggedLine2D As Line2d = New Line2d(New Point2d(jaggedLine.StartPoint(0), jaggedLine.StartPoint(1)),
                                                                        New Point2d(jaggedLine.EndPoint(0), jaggedLine.EndPoint(1)))
                                If jaggedLine2D.GetDistanceTo(New Point2d(ptStart.X, ptStart.Y)) < 1 And jaggedLine2D.GetDistanceTo(New Point2d(ptEnd.X, ptEnd.Y)) < 1 Then
                                    bJagged = True
                                End If
                            End If

                            'Call the extend API store the extended coords in array
                            Dim obj As Object
                            If bStretch = True Then
                                obj = line.IntersectWith(entForExtension, AcExtendOption.acExtendThisEntity)
                            Else
                                obj = line.IntersectWith(entForExtension, AcExtendOption.acExtendNone)
                            End If

                            If UBound(obj) > 0 Then
                                Dim ptExtendedPt As Point3d = New Point3d(Math.Round(obj(0)), Math.Round(obj(1)), obj(2))
                                'Check if point is on Slab outline and it is a stretch command then dont allow to stretch
                                'OR if point outside Slab outline and it is trim command then dont allow to trim
                                Dim res As PointContainment = getPointContainment(slabAreaPLine, ptExtendedPt)

                                'There was issue in trim in a rare scenario, where the intersection point is lying on the slab but result is giving it outsie
                                If res = PointContainment.Outside Then
                                    If bStretch = False Then
                                        Dim tol As Double = 0.4
                                        ptExtendedPt = New Point3d(obj(0), obj(1), 0)
                                        res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0), obj(1) + tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0), obj(1) - tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) + tol, obj(1), 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) - tol, obj(1), 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) + tol, obj(1) + tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) + tol, obj(1) - tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) - tol, obj(1) + tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                        If res = PointContainment.Outside Then
                                            ptExtendedPt = New Point3d(obj(0) - tol, obj(1) - tol, 0)
                                            res = getPointContainment(slabAreaPLine, ptExtendedPt)
                                        End If
                                    End If
                                End If

                                If res = PointContainment.Inside Then ' res = PointContainment.OnBoundary Or
                                    If bStretch = True Then
                                        GoTo donteditedge
                                    End If
                                ElseIf res = PointContainment.Outside Then
                                    If bStretch = False Then
                                        GoTo donteditedge
                                    End If
                                End If
                                Dim ptTmp As Point3d = New Point3d(0, 0, 0)
                                If (bStretch And ptStart.DistanceTo(ptExtendedPt) > ptEnd.DistanceTo(ptExtendedPt)) _
                                    Or (bStretch = False And (ptStart.DistanceTo(PointToTrim) > ptEnd.DistanceTo(PointToTrim))) Then
                                    'If end point is extended, the take edge from start to extended point
                                    curves.Add(New Line(ptStart, ptExtendedPt))

                                    'Update the next curve and it is curves array
                                    Dim nextline As AcadLine
                                    If ixs = UBound(explodedKr) Then
                                        nextline = explodedKr(0)
                                    Else
                                        nextline = explodedKr(ixs + 1)
                                    End If
                                    Dim ptNextEndpt As Point3d = New Point3d(nextline.EndPoint(0), nextline.EndPoint(1), 0)
                                    curves.Add(New Line(ptExtendedPt, ptNextEndpt))
                                    lstUpdatedLines.Add(nextline.Handle)
                                    ptTmp = ptNextEndpt

                                    'Update Span vector
                                    If bSpan = True Then
                                        updatedSpanLine = New Line(ptStart, ptExtendedPt)
                                    ElseIf bJagged = True Then
                                        jaggedLine = New Line(ptStart, ptExtendedPt)
                                    End If
                                Else
                                    'If start point is extended, the take edge from extended to end point
                                    curves.Add(New Line(ptExtendedPt, ptEnd))
                                    'Update the previous curve
                                    Dim prevline As AcadLine
                                    If ixs = 0 Then
                                        prevline = explodedKr(UBound(explodedKr))
                                    Else
                                        prevline = explodedKr(ixs - 1)
                                    End If
                                    Dim ptPrevstartpt As Point3d = New Point3d(prevline.StartPoint(0), prevline.StartPoint(1), 0)
                                    Dim ptPrevendpt As Point3d = New Point3d(prevline.EndPoint(0), prevline.EndPoint(1), 0)
                                    curves.Add(New Line(ptPrevstartpt, ptExtendedPt))
                                    lstUpdatedLines.Add(prevline.Handle)
                                    ptTmp = ptPrevendpt

                                    'Update Span vector
                                    If bSpan = True Then
                                        updatedSpanLine = New Line(ptExtendedPt, ptEnd)
                                    ElseIf bJagged = True Then
                                        jaggedLine = New Line(ptExtendedPt, ptEnd)
                                    End If
                                End If

                                If ptPrev = Point3d.Origin Then
                                    ptPrev = ptExtendedPt
                                Else
                                    If entForExtension.ObjectName = "AcDbPolyline" Then
                                        Dim ptTmpExtended As Point3d = New Point3d(ptExtendedPt.X, ptExtendedPt.Y, 0)
                                        Dim ptTmpPrev As Point3d = New Point3d(ptPrev.X, ptPrev.Y, 0)
                                        Dim pline As AcadLWPolyline = TryCast(entForExtension, AcadLWPolyline)
                                        GetCurvesWithinTwoExtendedPoints(curves, pline, ptTmpExtended, ptTmpPrev)
                                    Else
                                        curves.Add(New Line(ptPrev, ptExtendedPt))
                                    End If

                                    ptPrev = ptExtendedPt
                                End If
                                bIsSlabUpdated = True
                            Else
donteditedge:
                                ' If the line is on the side to trim then do not consider this line
                                If Not bIsVertical And Not bStretch Then
                                    ' get trim direction. +ve towards right i.e. right portion will be trimmed, -ve towards left
                                    ' the direction is calculated based on nearest point on entForExtension
                                    ' and the start point of slab outline line
                                    Dim dirVec As Integer = 0
                                    Dim tempLine As Line = Nothing
                                    Dim tempPLine As Polyline = Nothing
                                    Dim closestPt As Point3d
                                    If entForExtension.ObjectName = "AcDbPolyline" Then
                                        Dim pLine As AcadLWPolyline = TryCast(entForExtension, AcadLWPolyline)
                                        tempPLine = New Polyline()
                                        Dim coordinates As Object = pLine.Coordinates
                                        Dim verticesCount As Integer = (UBound(coordinates) + 1) / 2
                                        For index As Integer = 0 To verticesCount - 1
                                            tempPLine.AddVertexAt(index, New Point2d(coordinates(index * 2), coordinates(index * 2 + 1)), 0, 0, 0)
                                        Next

                                        tempPLine.Closed = True

                                        closestPt = tempPLine.GetClosestPointTo(PointToTrim, True)

                                    ElseIf entForExtension.ObjectName = "AcDbLine" Then
                                        Dim acdbLine As AcadLine = TryCast(entForExtension, AcadLine)
                                        tempLine = New Line(New Point3d(acdbLine.StartPoint(0), acdbLine.StartPoint(1), 0),
                                                            New Point3d(acdbLine.EndPoint(0), acdbLine.EndPoint(1), 0))
                                        closestPt = tempLine.GetClosestPointTo(PointToTrim, True)
                                    End If

                                    If tempLine IsNot Nothing Or tempPLine IsNot Nothing Then
                                        If Round(PointToTrim.X) - Round(closestPt.X) > 0 Then
                                            dirVec = 1
                                        ElseIf Round(PointToTrim.X) - Round(closestPt.X) < 0 Then
                                            dirVec = -1
                                        ElseIf Round(PointToTrim.Y) - Round(closestPt.Y) > 0 Then
                                            dirVec = 1
                                        ElseIf Round(PointToTrim.Y) - Round(closestPt.Y) < 0 Then
                                            dirVec = -1
                                        End If
                                       
                                        ...truncated...you should get the picture by now!


                lstVerticalLines.Clear()
            End If
            Return curves
        Catch ex As System.Exception

        End Try
    End Function


And yes, it's normal for my predecessors to not handle the exception and it's normal to see GoTo labels.  My predecessors were (and probably still are) morons....more accurately, this was written mostly by structural engineers who really should have stuck to their profession.  Please don't tell me I need to refactor this mess...I've got 2.5 million lines of code like this to look after between a VB.NET project and a Delphi project (please don't ask, I don't know why).  At least this function has comments ;-)

So, back to the original question, do I need to remove the line from the object collection before it gets reDIMed, or not?


MexicanCustard

  • Swamp Rat
  • Posts: 672
Re: Tidying up
« Reply #3 on: July 13, 2017, 08:49:19 am »
if you're worried about over allocation, DBObjectCollection is Disposable. So you can wrap the assignment in a using statement.
Code - C#: [Select]
  1. using(var curves = GetNewCurves(myDocuments, slabRegion, updatedSpanLine, localSpanVector, entForExtension, jaggedLine, slabAreaPLine, bStretch, PointToTrim, bIsSlabUpdated))
  2. {
  3.    //Use curves here
  4. }
  5.  

Quote
So, back to the original question, do I need to remove the line from the object collection before it gets reDIMed, or not?
No, since the variable is defined within the blocks of the foreach loop. A new variable is created for every iteration.  The previous variable goes out of scope every time you loop.
« Last Edit: July 13, 2017, 08:54:15 am by MexicanCustard »
AMEP 2017 64bit Win 10

n.yuan

  • Bull Frog
  • Posts: 232
Re: Tidying up
« Reply #4 on: July 13, 2017, 09:50:29 am »
So, back to the original question, do I need to remove the line from the object collection before it gets reDIMed, or not?

In classical VB (5/6/VBA) the minimal variable scope is limited within the Sub/Function. So, in VBA age, we constantly see a Sub/Function usually begins with quite a few lines of Dims to declare variables before any meaningful code.

However, in VB.NET (or C#), a variable's scope can be narrowed down to a logic code block, such as a For/While/Try...Catch/Using blocks. So, it is perfectly OK to declare variable inside a loop (I'd go further to say that it is even perferred. That is, it is preferred to only maintain a vriable in minimum scope necessary).

However, do not confuse variable scope and the notorious requirement in AutoCAD .NET API of disposing DBObject your code creates but is not added to database.

Yes, DBObjectCollection is disposable object, and you can use it with Using...End Using block. However, it is not an DBObject, and we do not have to explicitly dispose it. We can leave it for Garbage Collector to do its job. But if it contains DBObjects your code creates, then you need to deal with these DBObjects: either add them into database, or dispose them.

I did not take much time to read the code you posted, but it looks like a lot new entities (and even COM AcadEntity!) would be created just for calculating purpose, and they are not disposed. That is where the code could be problematic. So, even you said "do not tell you...", if I were you, I'd rather REFACTOR the code, at least the code you showed here.

pjm8765

  • Newt
  • Posts: 21
Re: Tidying up
« Reply #5 on: July 13, 2017, 11:28:53 am »
Thanks very much, most useful.