Anyway to improve this codes ?

[FoxWing]

Hi,

I'm curious is there anything i can do such as applying the codes below in a loop rather than what i've used below ? Most of the codes are identical except the cursor name and a condition. Imagine i have 10 cursors. I wouldn't want to replicate the codes 10 times.




strText = ''
SELECT * FROM CurPrint ORDER BY salestype WHERE printLoc1 = .T. into CURSOR CurPrint1
SELECT CurPrint1
SCAN
strText = Thisform.mthTextmerge(strText,item,variant)
ENDSCAN


strText = ''
SELECT * FROM CurPrint ORDER BY salestype WHERE printLoc2= .T. into CURSOR CurPrint2
SELECT CurPrint2
SCAN
strText = Thisform.mthTextmerge(strText,item,variant)
ENDSCAN



Any suggestion ?

Thanks.

 

[TheRambler]

Hi,

I would do it like this:

FOR i=1 TO 10
strText = ''
cField = "printLoc" + LTRIM(STR(i,2,0))
cCursor = "CurPrint" + LTRIM(STR(i,2,0))
SELECT * FROM CurPrint ORDER BY salestype ;
WHERE &cField = .T. into CURSOR &cCursor
SELECT &cCursor
SCAN
strText = Thisform.mthTextmerge(strText,item,variant)
ENDSCAN
ENDFOR

but I guess there must be a better way, (maybe a recursive function).

 

[TamarGranor]

Recursion doesn't make sense in this situation.

Tamar

 

[TheRambler]

Thanks for clarifying that. Recursion is something I still struggle to understand, hopefully someday I'll get it.

 

[Olaf Doschke]

Hi FoxWing,

that may be an issue of table normalisation:

All in all you do the same thing for records where any of the field printloc1 to printloc10 are .T., that's simply bad table design.

How to normalise the table depends upon if several of those fields can be .T. at the same record or only one field can be .T.. If only one, then make a field nPrintloc and store 1 to 10 to that, replacing the 10 fields.

If several of these fields can be .t. than you need an additional table with two fields, one having the primary key of curPrint to point back there and a second field that has the values 1 to 10 in it. Then you can store several records for each curPrint record to reflect the print locations. And again you can get rid of the 10 fields printloc1 to printloc10.

It's never a good idea to have fields with numbered names in a single table, that's a sign for bad table design.

Bye, Olaf.

 

[mkrausnick]

for i = 1 to 10
   fld="printloc"+ALLTRIM(STR(i))
   SELECT ... WHERE &fld. = .T. 
.
.
.
next i

should work.

And I disagree with Olaf that your table design is necessarily "bad". If you're a database design purist, maybe, but if you live in the real world, sometimes it's better to have a simple table design that violates the letter of the law but makes rapid data access possible using something less powerful than a Cray supercomputer.


Mike Krausnick
Dublin, California

 

[darrellblackhawk]

By the way, I don't see where TheRambler's code is
recursive. It's not calling itself; its just using
macro-substitution. TheRambler's code should work fine.

As a matter of fact, if the cursor quantity changes, you
can put it into a procedure.
i.e.

procedure SomeProcess
  lparam nCount
  local i, strText, cField, cCursor

  * Not sure where item and variant come from, but
  * if they're not part of cursors; which they seem
  * as if they are, they can be passed into the
  * procedure as well.

  for i = 1 to nCount
  
    strText = ''
    cField  = "printLoc" + ltrim(str(i,2,0))
    cCursor = "CurPrint" + ltrim(str(i,2,0))
    
    select * from CurPrint order by salestype ;
      where &cField = .t. into cursor &cCursor
      
    select &cCursor
    
    scan
      strText = thisform.mthTextmerge(strText,item,variant)
    endscan
    
  endfor
endproc

Darrell

 

[Olaf Doschke]

Mike,

such cases show, that some kind o pseudo array in a table is bad table design, not only for database purists.

What if printloc11,12,13,14 will be needed? That way you need ALTER TABLE, change the GUI to support that additional fields etc. That's also living in the "real world", nothing merely theoretical.

If you really want an array of boolean values you could have a field nPrintloc and set bits with BITSET and check bits with BITTEST etc. That way you could have 32 printlocs in one field and again wouldn't need makrosubstitution, but would filter the records for a certain printloc with WHERE BITTEST(nPrintloc,i) for values of i from 0 to 31.

And if only one of these fields can be .T., it definately is bad table design, because that information could also be put in one integer field, the number of the printloc, which would also simplify life with that data...

Bye, Olaf.

 

[mkrausnick]

Olaf,
I understand completely your point - I've sat on both sides of this discussion for over 20 years, and made the same points you make to programmers who wanted to take short-cuts. But sometimes there is a trade off between clean table design and usability in the sense of rapid data access.

Of course, a one-to-many Join table is the "correct" table structure for this task, but what if using a Join table makes the schema too difficult to parse with SQL?

And there's no difference between using an integer with BITSET/BITTEST and what Foxwing is already doing except the number of fields and the record size. How do you know Foxwing hasn't already built in extra fields for future expansion?

My point is we can't say what's the best or worst table design in this case because we can't see the entire schema.

Foxwing, sorry to digress - hopefully you find the discussion helpful or at least educational. One point Olaf made that you should definitely think about is that if there can be only one PRINTLOC, i.e., only one of the fields is true and the rest are false, then you should use a single integer or character field and not separate logical fields to identify it.




Mike Krausnick
Dublin, California

 

[TheRambler]

>I don't see where TheRambler's code is recursive.
Right, it is not. I was in a "recursive mood" that day..., while in another forum somebody asked if there was a way to get the sum of all the ones in a binary byte from a decimal value. For example, decimal 127 equals binary 01111111 whose sum of ones is seven.

I thought of two ways to do it, one with a loop and other using LEN(STRTRAN(cBinary, "0","")). But there were many other solutions I didn't think of, the one I liked the most was a recursive function using BITTEST() and BITRSHIFT().

Then I saw this question and wondered if it was possible too, to change the loop for a recursive function, but now I understand it doesn't make sense in this situation.

Also, I have a table structure like FoxWing's, except they are numeric fields which I must sum conditionally. So, I can clearly understand both, Olaf and Mike; if not anybody else, I find the discussion helpful. Thanks.

 

[Olaf Doschke]

Hi Mike,

Indeed we don't have enough insight to really say which teble design would be appropriate. At least a normalized design would help to simplify Foxwings original code.

And yes, the example I gave with Bittest() is not a big difference, but can also save much disk space, network bandwidth and thereby time. The new binary indices VFP9 offers may even make the 10 boolean printloc fields the better solution concerning the sql performance, but although I don't have the whole picture, my feeling about these fields is, that they are causing more trouble than comfort.

Bye, Olaf.

 

[darrellblackhawk]

Now that I really reviewed the postings, I'd have to agree
with the purists, that the current table structure is an
accident waiting to happen; unless there is no possibility
of needing more boolean test values in the future.

As MKrausnick stated, it will be best to use an integer to
denote printloc's.

After that, you can do something like the following:

for nPrintLoc = 1 to 10

  strText = ''

  select * from CurPrint ;
    where printLoc == nPrintLoc ;
    order by salestype ;
    into cursor CurPrintOut

  * select CurPrintOut
  * You don't need to re-select CurPrintOut

  scan
    strText = thisform.mthTextmerge(strText,item,variant)
  endscan

next

Darrell