ggreen86 / XLSX-Workbook-Class

VFP Class to Create an XLSX Workbook without Excel Automation or Installed
45 stars 16 forks source link

Create New Cursor #91

Closed SeregaKR closed 1 year ago

SeregaKR commented 1 year ago

Hello. I was checking the new beta (Release 39, beta 8). Everything works fine so far.

But I'd suggest making small optimization of the method CreateWorkingCursors. You have a lot of similar code with the CodePage number as the only difference. It'll be much easier to understand if you combine all code pages in one CASE and use a variable for inserting it into the creation of the cursors.

DO CASE
    CASE INLIST(this.CodePage, 737, 852, 857, 861, 865, 866, 874, 895, 932, 936, 949, 950) OR BETWEEN(this.CodePage, 1250, 1256)
        lnCodePage = thisCodePage &&add this variable and then insert it
        CREATE CURSOR xl_strings CODEPAGE = &lnCodePage (id I, workbook I, checksum C(230), stringxml M, stringval M, presvspace L, formatted L)
        CREATE CURSOR xl_cells CODEPAGE = &lnCodePage (workbook I, sheet I, cellrow I, cellcol I, cellvalue M, datatype C(1), formulatype C(10), formulasi I, ;
            stringid I, cellxfs I, numdec I, celldeleted L, validndx I, ignorwarn I)
        CREATE CURSOR xl_sheets CODEPAGE = &lnCodePage (workbook I, sheet I, shname C(30), state I, gridlines I, mleft N(6,3), mright N(6,3), mtop N(6,3), mbot N(6,3), mheader N(6,3), ;
            mfooter N(6,3), shdeleted L, xsplit I, ysplit I, prnorient I, papersize I, paperwidth I, paperheight I, paperdimen C(2), scale I, fittowidth I, fittoheight I, ;
            tabcolorndx I, tabcolorrgb C(8), outline L, grpsumbelow I, grpsumright I, protection L, password C(50), algorithm C(10), autofilter L, deletecol L, ;
            deleterow L, formatcell L, formatcol L, formatrow L, insertcol L, insertrow L, inserthyper L, objects L, pivottbl L, scenarios L, sellocked L, selunlocked L, ;
            sort L, zoomscale I, prnopthead I, prnoptgrid I, draft I, blackwhite I, pagescale I, fittopage I, toplfcell C(20), ;
            tabselctd I, activcell C(20), prnoptctrv I, prnoptctrh I)
    OTHERWISE
ggreen86 commented 1 year ago

Sergey—

Thank you for feedback and the suggestion. Not sure how much it would actually save. The trade-off between the CASE statement and the macro substitution would probably be nearly the same. However, it would make the code a bit easier to maintain when I change a cursor to add a field…

Greg

From: Sergey @.> Sent: Friday, June 16, 2023 7:24 AM To: ggreen86/XLSX-Workbook-Class @.> Cc: Subscribed @.***> Subject: [ggreen86/XLSX-Workbook-Class] Create New Cursor (Issue #91)

Hello. I was checking the new beta (Release 39, beta 8). Everything works fine so far.

But I'd suggest making small optimization of the method CreateWorkingCursors. You have a lot of similar code with the CodePage number as the only difference. It'll be much easier to understand if you combine all code pages in one CASE and use a variable for inserting it into the creation of the cursors.

DO CASE

    CASE INLIST(this.CodePage, 737, 852, 857, 861, 865, 866, 874, 895, 932, 936, 949, 950) OR BETWEEN(this.CodePage, 1250, 1256)

           lnCodePage = thisCodePage &&add this variable and then insert it

           CREATE CURSOR xl_strings CODEPAGE = &lnCodePage (id I, workbook I, checksum C(230), stringxml M, stringval M, presvspace L, formatted L)

           CREATE CURSOR xl_cells CODEPAGE = &lnCodePage (workbook I, sheet I, cellrow I, cellcol I, cellvalue M, datatype C(1), formulatype C(10), formulasi I, ;

                   stringid I, cellxfs I, numdec I, celldeleted L, validndx I, ignorwarn I)

           CREATE CURSOR xl_sheets CODEPAGE = &lnCodePage (workbook I, sheet I, shname C(30), state I, gridlines I, mleft N(6,3), mright N(6,3), mtop N(6,3), mbot N(6,3), mheader N(6,3), ;

                   mfooter N(6,3), shdeleted L, xsplit I, ysplit I, prnorient I, papersize I, paperwidth I, paperheight I, paperdimen C(2), scale I, fittowidth I, fittoheight I, ;

                   tabcolorndx I, tabcolorrgb C(8), outline L, grpsumbelow I, grpsumright I, protection L, password C(50), algorithm C(10), autofilter L, deletecol L, ;

                   deleterow L, formatcell L, formatcol L, formatrow L, insertcol L, insertrow L, inserthyper L, objects L, pivottbl L, scenarios L, sellocked L, selunlocked L, ;

                   sort L, zoomscale I, prnopthead I, prnoptgrid I, draft I, blackwhite I, pagescale I, fittopage I, toplfcell C(20), ;

                   tabselctd I, activcell C(20), prnoptctrv I, prnoptctrh I)

    OTHERWISE

— Reply to this email directly, view it on GitHubhttps://github.com/ggreen86/XLSX-Workbook-Class/issues/91, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGWB33KQA3RQPRPJNPBZFKDXLQ637ANCNFSM6AAAAAAZJD275U. You are receiving this because you are subscribed to this thread.Message ID: @.**@.>>

ggreen86 commented 1 year ago

I have incorporated your suggestion into the CreateWorkingCursors() method. However, I did not use macro substitution as your example, but instead enclosed the CodePage setting in parenthesis as shown:

CREATE CURSOR xl_strings CODEPAGE = (this.CodePage) (id I, workbook I, checksum C(230), stringxml M, stringval M, presvspace L, formatted L)