Quick on my computer slow on another 3

[Steve-vfp9user]

Hello all,

The following code is used to store a product number/code from a table PRODLIST then search for that code in customer orders that have been archived (ARELATEPRODS) or are current (RELATEPRODS). The purpose of this is to find the last date the product was sold. As I have inherited this, it would have been more efficient st the time of sale, if the actual product was updated back to the PRODLIST table but this isn't the case.

Here's the code:

USE PRODLIST SHARED
GO TOP

DO WHILE NOT EOF()
  STORE CTOD("  /  /    ") TO mlastsold
  mprodlistrecno=RECNO()
  GO mprodlistrecno
  mprodnumb=0
  mprodnumb=PRODNUMB

  USE ARELATEPRODS SHARED && Archived Records
  GO TOP
  SCAN
    IF PRODNUMB=mprodnumb
      IF INVDATE>mlastsold
        STORE INVDATE TO mlastsold
      ENDIF
    ENDIF
  ENDSCAN

  USE RELATEPRODS SHARED && Archived Records
  GO TOP
  SCAN
    IF PRODNUMB=mprodnumb
      IF INVDATE>mlastsold
        STORE INVDATE TO mlastsold
      ENDIF
    ENDIF
  ENDSCAN
	
* Replace the LASTSOLD field with the date

  USE PRODLIST SHARED
  GO mprodlistrecno
  REPLACE LASTSOLD WITH mlastsold
  SKIP
  IF EOF()
    EXIT
  ENDIF
ENDDO

Is there a more efficient way to do this?

Thank you

Steve Williams
VFP9, SP2, Windows 10

 

[GriffMG]

Yes, you could keep all the tables open and not need to keep reopening them/finding your place...

SELECT 0
USE PRODLIST SHARED
SELECT 0
USE ARELATEPRODS SHARED && Archived Records
SELECT 0
USE RELATEPRODS SHARED && Archived Records

SELECT PRODLIST
Go TOP
DO WHILE .NOT. EOF()
  STORE CTOD("  /  /    ") TO mlastsold
  mprodnumb=PRODNUMB

  SELECT ARELATEPRODS 
  GO TOP
  SCAN
    IF PRODNUMB=mprodnumb
      IF INVDATE>mlastsold
        STORE INVDATE TO mlastsold
      ENDIF
    ENDIF
  ENDSCAN

  SELECT RELATEPRODS 
  GO TOP
  SCAN
    IF PRODNUMB=mprodnumb
      IF INVDATE>mlastsold
        STORE INVDATE TO mlastsold
      ENDIF
    ENDIF
  ENDSCAN
	
* Replace the LASTSOLD field with the date

  SELECT PRODLIST 
  REPLACE LASTSOLD WITH mlastsold
  SKIP
ENDDO 

SELECT ARELATEPRODS 
use
SELECT RELATEPRODS 
use
SELECT PRODLIST 
USE

There is lots more you could do, index the related products tables and seek rather than scan.




Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are !good for you. ​

 

[Steve-vfp9user]

Thank you Griff

I'll run that and see how it goes. I'll also change it slightly as you've suggested with SEEK etc and post back.

Thank you

Steve Williams
VFP9, SP2, Windows 10

 

[Mike Lewis]

At the very least, I would replace this:

SELECT ARELATEPRODS 
  GO TOP
  SCAN
    IF PRODNUMB=mprodnumb
      IF INVDATE>mlastsold
        STORE INVDATE TO mlastsold
      ENDIF
    ENDIF
  ENDSCAN

with this:

SELECT MAX(Invdate) FROM ARELATEPRODS WHERE PRODNUMB=mprodnumb ;
  INTO ARRAY laTemp
MLastSold = laTemp(1)

and similarly for RELATEPRODS.

You should also ensure that there is an index on Prodnum in the two tables. That way, the code will be optimised. Your original code, on the other hand, will be slower because it has to visit every record in the two tables.

And I definitely agree with Griff that you shouldn't be constantly opening the tables. Open them once, at the start of the routine, and leave them open until the end.

Mike

Mike




__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Steve-vfp9user]

Mike

Much appreciated as always and again, something else to work with.

Thank you

Steve Williams
VFP9, SP2, Windows 10

 

[GriffMG]

Mike I think you would need to test/qualify that last assignment.

if laTemp(1) > MLastSold
  MLastSold = laTemp(1) 
Endif

Regards

Griff
Keep [Smile]ing

There are 10 kinds of people in the world, those who understand binary and those who don't.

I'm trying to cut down on the use of shrieks (exclamation marks), I'm told they are !good for you. ​

 

[Mike Lewis]

You're right of course, Griff. Or you could also do:

MLastSold = MAX(MLastSold, latemp(1))

I might also include a test for [tt]_TALLY > 0[/tt], given that, if no matching product is found, the array element would be NULL, which would probably muck things up down the road.

Mike

__________________________________
Mike Lewis (Edinburgh, Scotland)

Visual FoxPro articles, tips and downloads

 

[Olaf Doschke]

You already begin with some things you could simply skip to do:

mprodlistrecno=RECNO()
  GO mprodlistrecno

This GO goes to the same record you already are at. The only context this makes sense is to flush changes, but you opened the table up at the start, there are no buffered changes from this users session and you don't trigger other application users to flush their changes by GO to some record. Overall to go through all records you use SCAN..EDSCAN as your code also does in some places and even if you use a DO WHILE !EOF() you only need GO TOP before the loop and SKIP 1 at the end. There is no need to GO to a record number and especially not to the one you just read from the current position via RECNO(), if there is a need to go back to a previously remembered RECNO(), then it's after some SEEK, LOCATE or FILTER or any other operation changing record position to reset it.

mprodnumb=0
mprodnumb=PRODNUMB

There is no need to set a variable value twice, or to zero it before writing the next value to it.

I'll stop at this point because all you're interested in is the max invdate from the two tables ARELATEDPRODS and RELATEDPRODS for each PRODNUMB and to store that into PRODLIST.LASTSOLD. Overall, that asks for an UPDATE from a SELECT ... GROUP BY PRODNUMB. If you know SQL you can do lots of stuff not only in a single query, albeit more complex in code, but also much faster.

If you can do things faster the xbase way, then only by SEEKs and you don't even use a FOR condition in your SCANs, if the SCAN loop code body is an IF statement about a field of the scanned workarea, then make this a FOR condition, you can skip an IF this way.

First ,see if this list reflects the correct max invdates:

SELECT PRODLIST.PRODNUMB, MAX(NVL(MAX(ARELATEDPRODS.INVDATE),{}),NVL(MAX(RELATEDPRODS.INVDATE),{})) as LASTSOLD;
FROM PRODLIST;
LEFT JOIN ARELATEPRODS ON ARELATEPRODS.PRODNUMB=PRODLIST.PRODNUMB;
LEFT JOIN  RELATEPRODS ON  RELATEPRODS.PRODNUMB=PRODLIST.PRODNUMB;
GROUP BY PRODLIST.PRODNUMB;
INTO CURSOR crsLastSold

Indexes on PRODNUMB and INVDATE in all tables having that field are required to optimize this. And once this list is correct, it's easy to turn this into an UPDATE of the LASTSOLD column.

Bye, Olaf.

Olaf Doschke Software Engineering

https://www.doschke.name

 

[Steve-vfp9user]

Olaf

Your input is also much appreciated and valued. I’ll mull over the posts and see what I come up.

Thank you

Steve Williams
VFP9, SP2, Windows 10