On Topic: Help with coding problem

[d...@computerconsult.com]

I appologize for the word wrap and complexity in trying to read this
code sample. I am generating a 5333 error in this method when process
larger files (250,000 records or more). As a requirement for this
project we have a module that allows a user to map fields from a source
file into a target file for import purposes. You will note that we
have given them an option to create a codeblock to modify data coming
from the source to the target. For example you might want sourcefield1
to go to targetfield8 and to substr(sourcefield1,5,10) in the process.
The block of code starting " FOR i := 1 UPTO ALen(aMapA) STEP 1" is
where the macro compile of these codeblocks takes place.

Now for anyone who is still reading, I need to fix this code, short of
start over and anyone suggest how I might attempt to get rid of the
5333 error?

METHOD SubPBPreview CLASS MyMapper
LOCAL oDSTar AS DBServer, w AS INT
LOCAL oDSSou AS DBServer, aCBS AS USUAL, sTempString AS USUAL
LOCAL i AS INT, x AS STRING, y AS INT, z AS INT
LOCAL aSourz AS ARRAY, DTy AS STRING, DTy2 AS STRING
LOCAL TFC AS INT, LAS AS INT, TotRecs AS DWORD, TotDone AS DWORD
LOCAL aMapA AS ARRAY, b AS STRING, cStuff AS STRING,
LOCAL nStuff AS FLOAT, lStuff AS LOGIC, dStuff AS DATE, uStuff AS
USUAL

SELF:Pointer := Pointer{ POINTERHOURGLASS }

aMapA := {}

SELF:oSFSub_Form2:Server:GoTop()
DO WHILE ! SELF:oSFSub_Form2:Server:Eof
AAdd(aMapA,
SELF:oSFSub_Form2:Server:FNTARGET,SELF:oSFSub_Form2:FNYESNO,SELF:oSFSub_Form2:FNSource,SELF:oSFSub_Form2:CODESTREAM})
SELF:oSFSub_Form2:Server:Skip( 1 )
ENDDO

oDSTar := DBServer{"IOFile.dbf",FALSE,FALSE,"DBFCDX"}
oDSTar:Zap()

oDSSou := DBServer{SELF:PullFrom,,TRUE,TRUE,"DBFCDX"}
aSourz := oDSSou:DBSTRUCT
oDSSou:GoBottom()

TFC := oDSTar:FCount
TotRecs := oDSSou:RecNo
TotDone := 0

FOR i := 1 UPTO ALen(aMapA) STEP 1
IF ! Empty(aMapA[i,4]) // code block or expression
sTempString := StrTran(aMapA[i,4],"{S}","s:") // convert source to
class instance
sTempString := StrTran(sTempString,"{T}","t:") // convert target to
class instance
sTempString := "{|s,t| "+AllTrim(sTempString)+"}"
aCBS := MCompile(sTempString)
IF ! Empty(aCBS) // aCBS = NULL_STRING
aMapA[i,4] := aCBS
ENDIF
ENDIF
NEXT

aCBS := ""
sTempString := ""

oDSSou:GoTop()
DO WHILE ! oDSSou:EoF

oDSTar:Append()

DynLock()

FOR i := 1 UPTO TFC STEP 1
y := 0
b := oDSTar:FieldName(i)
w := AScan(aMapA, {|aVal| aVal[1] = b}) // find map data for the
field

DO CASE //"FNTarget"
CASE w > 0
x := aMapA[w,3] // source
y := 0
LAS := ALen(aSourz)

FOR z := 1 UPTO LAS STEP 1
IF Trim(aSourz[z,1]) == Trim(x)
y := z
ENDIF
NEXT

DO CASE
CASE y > 0
DTy := oDSTar:FieldInfo(DBS_TYPE,i)
DTy2 := oDSSou:FIELDInfo(DBS_TYPE,y)
DO CASE
CASE ! Empty(aMapA[w,4])
uStuff := Eval(MExec(aMapA[w,4]),oDSSou,oDSTar)
oDSTar:FIELDPUT(i,uStuff)
CASE DTy = 'C'
DO CASE
CASE DTy2 = 'N'
cStuff := Str(oDSSou:FIELDGET( y ))
CASE DTy2 = 'D'
cStuff := DToC(oDSSou:FIELDGET( y ))
CASE DTy2 = 'L'
cStuff := iif(oDSSou:FIELDGET( y ),"Y","F")
OTHERWISE
cStuff := oDSSou:FIELDGET( y )
ENDCASE
oDSTar:FIELDPUT(i,cStuff)
CASE DTy = 'N'
DO CASE
CASE DTy2 = 'C'
nStuff := Val(oDSSou:FIELDGET( y ))
CASE DTy2 = 'D'
nStuff := 0
CASE DTy2 = 'L'
nStuff := 0
OTHERWISE
nStuff := oDSSou:FIELDGET( y )
ENDCASE
oDSTar:FIELDPUT(i,nStuff)
CASE DTy = 'D'
DO CASE
CASE DTy2 = 'C'
dStuff := CToD(oDSSou:FIELDGET( y ))
CASE DTy2 = 'N'
dStuff := CToD(Space(8))
CASE DTy2 = 'L'
dStuff := CToD(Space(8))
OTHERWISE
dStuff := oDSSou:FIELDGET( y )
ENDCASE
oDSTar:FIELDPUT(i,dStuff)
CASE DTy = 'L'
DO CASE
CASE DTy2 = 'C'
lStuff := iif(oDSSou:FIELDGET( y )="Y",TRUE,FALSE)
CASE DTy2 = 'D'
lStuff := FALSE
CASE DTy2 = 'N'
lStuff := FALSE
OTHERWISE
lStuff := oDSSou:FIELDGET( y )
ENDCASE
oDSTar:FIELDPUT(i,lStuff)
OTHERWISE
Errorbox{SELF,"Undefined data type"}:Show()
ENDCASE
ENDCASE
ENDCASE
NEXT

DynUnLock()

TotDone := TotDone + 1
IF MOD(TotDone,1000) = 0
oDSTar:Commit()
//CollectForced()
ENDIF
oDSSou:Skip( 1 )
ENDDO

oDSTar:Commit()
oDSSou:Close()
oDSTar:Close()

SELF:Pointer := Pointer{ POINTERARROW }

RETURN NIL

[Marc Verkade [Marti IT]]

Heavy array stuff does need a ocasional

If Mod(x,100)==0
If !InCollect()
CollectForced()
End
End

I saw that the Collectforced was comented out..
Perhaps you can try that somewhere in te code.
Grtz, Marc

<d...@computerconsult.com> schreef in bericht
news:1161214860.0...@k70g2000cwa.googlegroups.com...

[d...@computerconsult.com]

Thanks Mark,

I had the collectforced in there but the problem wasn't solved. I am
hoping someone can help me find a better way to deal with the macro
portion, or spot the true problem (no pot shots at VO here, <g>).

Dan
"To thine own self be true, no one else will"

[Stephen Quinn]

Dan

You do no testing to see if the DBF exists or if you open it sucessfully

> oDSTar := DBServer{"IOFile.dbf",FALSE,FALSE,"DBFCDX"}
> oDSTar:Zap()

Tooooo many parameters here

> oDSSou := DBServer{SELF:PullFrom,,TRUE,TRUE,"DBFCDX"}
> aSourz := oDSSou:DBSTRUCT
> oDSSou:GoBottom()

> FOR i := 1 UPTO ALen(aMapA) STEP 1
FOR i := 1 UPTO ALen(aMapA) // No need for the STEP as UPTO defaults to 1
You also do no testing to see if aMapA isn't empty()

Might help if you also surround the code with
oServer:SuspendNotification/oServer:ResetNotification()

> TotDone := TotDone + 1
> IF MOD(TotDone,1000) = 0
> oDSTar:Commit()
> //CollectForced()
> ENDIF

Waste of time - every time you APPEND() a new record the previous one gets
committed automatically, you only need the one at the end of your loop - if you
open the servers exclusively then commits aren't required.

// Some simplified code - makes things easier to read & see any mistakes<G>

DO WHILE ! oDSSou:EoF

oDSTar:Append()
// DynLock()
FOR i := 1 UPTO TFC // STEP 1

y := 0
b := oDSTar:FieldName(i)
w := AScan(aMapA, {|aVal| aVal[1] = b}) // find map data for the

FIELD

DO CASE //"FNTarget"
CASE w > 0
x := aMapA[w,3] // source
y := 0
LAS := ALen(aSourz)

FOR z := 1 UPTO LAS // STEP 1

IF Trim(aSourz[z,1]) == Trim(x)
y := z
ENDIF
NEXT

// You assume that a field appears in both DBFs (y>0)
// What if F1(FRED) is called F2(FIDO) in the second DBF (user
wants to map to different field)
// y = 0 in that case and the data never gets mapped/copied at
all

DO CASE
CASE y > 0

cTargetType := oDSTar:FieldInfo( DBS_TYPE, i )
cSourceType := oDSSou:FIELDInfo( DBS_TYPE, y )
xSourceData := oDSSou:FIELDGET( y )

DO CASE
CASE ! Empty(aMapA[w,4])
uStuff := Eval(MExec(aMapA[w,4]), oDSSou, oDSTar )
oDSTar:FIELDPUT(i,uStuff)

CASE cSourceType == cTargetType
// More things need to be checked here for numerics
especially
// fieldsize/ numeric size & decimals
oDSTar:FIELDPUT( i, xSourceData )

OTHERWISE
xTargetData := SELF:ConvertIt( xSourceData, cSourceType,
cTargetType )
oDSTar:FIELDPUT( i, xTargetData )
ENDCASE

ENDCASE
ENDCASE
NEXT

// DynUnLock()

oDSSou:Skip( 1 )
ENDDO

oDSTar:Commit()
oDSSou:Close()
oDSTar:Close()

SELF:Pointer := Pointer{ POINTERARROW }

RETURN NIL

METHOD ConvertIt( xData, cSType, cTType ) CLASS Whatever

// I'll have to assume there are no memos involved
DO CASE
CASE cTType == 'C'

DO CASE
CASE cSType == 'N'
xData := NTrim( xData )

CASE cSType == 'D'
xData := DToC( xData )

CASE cSType == 'L'
xData := IIF( xData ),"Y","F")
ENDCASE

CASE cTType == 'D'

IF cSType2 == 'C'
xData := CToD( xData )
ELSE
xData := CToD('')
ENDIF

CASE cTType == 'N'

IF cSType2 == 'C'
// More things need to be checked here
// fieldsize / numeric size & decimals
xData := Val( xData )
ELSE
xData := 0
ENDIF

CASE cTType == 'L'

IF cSType == 'C'
// Assumes a lot about the Source data
xData := IIF( xData == 'Y', TRUE, FALSE )
ELSE
xData := FALSE
ENDIF

ENDCASE

RETURN xData

HTH
Steve

[d...@computerconsult.com]

Stephen,

Thanks for your input. I am going to try a few of your suggestions to
see what impact it has. As far as the need for more error checking and
such, I actually pulled that out of this sample to try and make it
smaller and to get down to the real problem code. This is VERY UGLY
code, but it is also quite abstract and probably why I am struggling
with it. I appreciate your comments and welcome any other ideas as
well.

Dan

[Werner Perplies]

Am 18 Oct 2006 16:41:00 -0700 schrieb d...@computerconsult.com:

[...]

I haven't check your complete code, but I've seen a aadd() in it. I avoid
this with arrays of a big number of elements:

I use instead the construct in the function CreateArrayWithOutAadd(aArray,
dwArraySize)

See the following test application:

FUNCTION Start()

LOCAL dwArraySize AS DWORD
LOCAL dwElements AS DWORD
LOCAL aArray AS ARRAY

dwArraySize := 199790
aArray := {}
CreateArrayWithOutAadd(aArray, dwArraySize)
dwElements := ALen(aArray)
aArray := {}
CreateArrayWithAadd(aArray, dwArraySize)
dwElements := ALen(aArray)
aArray := {}

RETURN
FUNCTION CreateArrayWithOutAadd(aArray AS ARRAY, dwArraySize AS DWORD) AS
VOID PASCAL

LOCAL dwNextNumberOfElements AS DWORD
LOCAL dwElement AS DWORD
LOCAL dwActualElements AS DWORD
LOCAL I AS DWORD

dwNextNumberOfElements := 128
aArray := ASize(aArray,dwNextNumberOfElements)
dwActualElements := dwNextNumberOfElements
dwElement := 0
DO WHILE TRUE

IF dwActualElements < ++dwElement

dwActualElements += dwNextNumberOfElements
ASize(aArray,dwActualElements)

ENDIF // dwActualElements < I
aArray[dwElement] := {"Element: " , NTrim(dwElement)}
IF dwElement = dwArraySize

EXIT

ENDIF // dwElements = dwArraySize
ENDDO // While TRUE

ASize(aArray,dwElement)

RETURN
FUNCTION CreateArrayWithAadd(aArray AS ARRAY, dwArraySize AS DWORD) AS VOID
PASCAL

LOCAL dwElement AS DWORD

dwElement := 0
DO WHILE TRUE

++dwElement
AAdd(aArray,{"Element: " , NTrim(dwElement)})
IF dwElement = dwArraySize

EXIT

ENDIF // dwElements = dwArraySize

ENDDO // While TRUE

RETURN

Werner
--
German phpBB-Board for Visual Objects user:
http://www.weepee.eu/forum/vo/
divUtilies: new version 14th June, 2006
http://www.weepee.eu/vo/DivUtilities/DivUtilities.zip

[d...@computerconsult.com]

Werner,

Thanks for the idea. I replaced the aadd(). Specifically a changed
the declaration for aMapA to "Local aMapA[200,4] as Usual". Then I
used aSize(aMapA,RecCount), and just stored the values from the server
into the array elements. It ain't fast or pretty, but it ran without
error.

I think I will put this in place to keep the client happy, and get busy
trying to tighten this code up.

Dan

[Werner Perplies]

Dan,

Am 18 Oct 2006 22:03:06 -0700 schrieb d...@computerconsult.com:

> Werner,
>
> Thanks for the idea. I replaced the aadd(). Specifically a changed
> the declaration for aMapA to "Local aMapA[200,4] as Usual". Then I
> used aSize(aMapA,RecCount), and just stored the values from the server
> into the array elements. It ain't fast or pretty, but it ran without
> error.

I'm not sur, if I understood right, but I found (2.5b3), this code is much
faster than aadd().

The speed depends only a little bit of size of dwNextNumberOfElements.

[Stephen Quinn]

Dan

"Local aMapA[200,4] as Usual"

Should be type ARRAY not USUAL

That declaration is (slightly) incorrect if your still using
aMapA[ i ] := { the 4 element array you were aadd()ing before }
then there is no need, for the second parameter to the array declaration as it
gets blow away when you assign the data array to aMapA[ i ]

Unless you've changed your code to add each sub-element individually that is<g>
Eg
i := 0
Do WHILE ! SELF:oSFSub_Form2:Server:EOF
++i
aMapA[ i, 1 ] := SELF:oSFSub_Form2:Server:FNTARGET
aMapA[ i, 2 ] := SELF:oSFSub_Form2:Server:FNYESNO
aMapA[ i, 3 ] := SELF:oSFSub_Form2:Server:FNSource
aMapA[ i, 4 ] := SELF:oSFSub_Form2:Server:CODESTREAM
SELF:oSFSub_Form2:Server:Skip()
ENDDO

Much simpler to just
aMapA := ArrayNew( SELF:oSFSub_Form2:Server:RecCount() )
instead of
aMapA := {}
or
aMapA[200,4]
aSize(aMapA,RecCount)

Why do something twice when you can do it once and end up with the same result.

HTH
Steve

[ant...@gmail.com]

Hi Dan,

With such large arrays, and heavy duty work you should not be using
arrays.
Take a look at RamDbf2, or DataSet in Antlib 1.3 (No ODBC).
You can download if from www.knowvo.com there are samples on how to use
the classes.
These will address your crashes.
Also, with string manipulation, I would take a look at the
Stringbuilder, you are generating a LOT of garbage collection calls.
that will also help.

Cheers Anthony

d...@computerconsult.com schreef:

[d...@computerconsult.com]

Just to benefit anyone that may be dealing with a similar issue in the
future, here is what worked.

I changed the scope of the oDSSou and oDSTar servers to class
variables, as well as moving the array handling the codeblocks to a
dbserver. Changing the scope of the two servers seemed to provide the
most dramatic impact on the problem. The new routine works and seems
to be pretty bullet proof. It is slower, although I am taking some
steps to improve the performance.

The truth of the matter is that apparently in the codeblock that was
being mcompiled the servers were being lost (garbage collection, or
something) changing their scope eliminated this from happening.

Thanks again for all the great input.

Dan

[Sherlock]

Dan

snip[ module that allows a user to map fields from a source file into a
target file for import purposes ]

FOR xx := 1 UPTO nFields
IF oBookHist:FieldPos(oBookings:FieldSym(xx)) > 0
oBookHist:FIELDPUT(oBookings:FieldSym(xx),
oBookings:FIELDGET(xx) )
ENDIF
NEXT

You might find this easier than the AScan as it test for the FIELD in
the receiving database and allows for its position automatically.

Phil McGuinness
----

[Werner Perplies]

Dan,

Am 18 Oct 2006 16:41:00 -0700 schrieb d...@computerconsult.com:

Another thing, which can make you code faster:

b := oDSTar:FieldName(i)

AScan(aMapA, {|aVal| aVal[1] = b}) // find map data for the

Not looked, where you store this data, but I think, it would be better to
store Filedname as Symbol and than scan for the symbol, this shold be
faster.