Re: [WebERP-developers] CustomerInquiry.php W3C html code.

[Phil D. <ph...@lo...>]

Thanks Tim,

Looks good to me
Not sure why you haven't committed this yourself though? Let me know if 
you want me to do it for you?
We've had this discussion before and it gets boring for everyone .... 
you still have access to the code.



On the abstraction issue... yes some abstraction makes sense and we have 
plenty already. I am not into making a function for every little bit of 
html we echo though as I feel the presentation code adds context. I 
don't wish to debate this ... again!
A common customer search function though might be a useful in 
SQL_CommonFunctions.inc

Phil

Phil Daintree
Logic Works Ltd - +64 (0)275 567890
http://www.logicworks.co.nz

On 19/09/14 04:13, Tim Schofield wrote:
> While waiting for Phil's ruling on this I have redone
> CustomerInquiry.php in what I think is a more readable way, that
> always has the same number of columns no matter what the transaction,
> and changed the original code to comply with the guidelines.
>
> Am attaching the new script,
> Tim
>
> On 17 September 2014 12:15, Tim Schofield <tim...@gm...> wrote:
>> Hi Phil, still no polite or useful reply to this. I think it is
>> important for us developers to have a ruling on what can and what
>> cannot be abstracted.
>>
>> Yours in anticipation,
>> Tim
>>
>> On 16 September 2014 11:05, Tim Schofield <tim...@gm...> wrote:
>>> Phil,
>>>
>>> Do you stand by your comments that the stock search should be
>>> abstracted to a function in SQLCommonFunctions.inc?
>>>
>>> Thanks
>>> Tim
>>>
>>> On 16 September 2014 09:53, Phil Daintree <ph...@lo...> wrote:
>>>> Hola Rafael,
>>>>
>>>> As we have corresponded off list, I know that these comments within will
>>>> be no surprise to you - but for the benefit of anyone else.
>>>>
>>>>
>>>> On 16/09/14 07:31, Rafael Chacón wrote:
>>>>> Hi All,
>>>>>
>>>>> *** ABOUT CustomerInquiry.php: ***
>>>>>
>>>>> 1. The problem: In some web-browser, incomplete table-rows
>>>>> ("<tr></tr>" tags with less quantity of columns) causes an unexpected
>>>>> behaviour (e.g. blanks and misaligned columns). E.g. Android
>>>>> 3.0.31/Navigator (Personal comment: I promote this ERPs as an ERP that
>>>>> can be run in a small device and can be run anywhere with a slow
>>>>> Internet connection, therefore it must run fine and fast).
>>>>>
>>>> Well I am not sure all the screens are so good on smaller devices.
>>>>
>>>>
>>>>> 2. The goal: fix this problem, document it, and have a code easy to
>>>>> understand for a newcomer (Personal comment: the last is the most
>>>>> difficult task. We can not "reset" our memory about the code to test
>>>>> if it is easy to understand for a newcomer).
>>>>
>>>> Yes it is subjective - but consistency with existing code and strict
>>>> adherence to the conventions/style as per
>>>>
>>>> http://www.weberp.org/CodingConventions.html
>>>>
>>>> should cover it.
>>>>
>>>>> 3. What I did: I decomposed the code to understand it. After that, I
>>>>> reassembled. The result is one section with the constant part [format
>>>>> string and other components like html-links and called parameters
>>>>> (names) ]; that is BaseInfo, CreditInvoice, Allocation,
>>>>> PreviewInvoice, PreviewCredit and GLEntries section. And an other
>>>>> section with the variable part ("parameters"), where we can see the
>>>>> table-row structure. The main problem is still make evident the HTML
>>>>> table structure.
>>>>>
>>>> The problem with that though is that is that it ended up with a LOT of
>>>> changed code and the actual fix was obscured. Much better I think to
>>>> evolve with just the code necessary to effect the bug fix
>>>>
>>>>
>>>>> 4. Secondary effects:
>>>>>
>>>>> 4.1. Regroups and comments the initial basic lines of script
>>>>> (includes/session.inc, $Title, $ViewTopic, $BookMark,
>>>>> includes/header.inc). Even without much impact, the purpose is to
>>>>> ensure that these lines are uniformly in all scripts that require it.
>>>>>
>>>> All great contributions these.
>>>>
>>>>
>>>>> 4.2. Deletes $tableheader variable. This variable is defined in line
>>>>> 253 and only used in next line (line 271). Nowhere else is used that
>>>>> variable. I think it's better to remove it to simplify the code and,
>>>>> theoretically(*), to become more fast the code.
>>>>>
>>>> Yep - if used several times it messes with the table sorting javascript
>>>> anyway.
>>>>
>>>>> 4.3. Regroups some echo string functions. Instead of some consecutive
>>>>> echo language constructors, regroup them in only one instruction to
>>>>> simplify code. I think it makes the code easier to understand.
>>>>>
>>>> Me too - probably reduces the work of the parser too.
>>>>
>>>>> 4.4. Adds the ascending class to Data, Type, Number and Reference
>>>>> columns. I found that some people look for an operation (line) seeking
>>>>> it by transaction date (over half of the time), type, number and
>>>>> reference (by customer's purchase order reference or by payment
>>>>> reference). This is a new feature to help users, and it is a lack if
>>>>> we compare CustomerInquiry.php with SupplierInquiry.php
>>>> Yep  worthwhile for most scripts
>>>>> 4.5. $RootPath and $Theme in the "Constant Section". Those variables
>>>>> always hold the same values throughout the time this script runs.
>>>>> For clarity, I think it is better to separate those "constant
>>>>> variables" from the each row variables (printf parameters section).
>>>>>
>>>> Not quite sure what you mean here?
>>>>
>>>>> 5. Help wanted:
>>>>>
>>>>> 5.1. "Constant Section" variable names. Because those variables
>>>>> (BaseInfo..., CreditInvoice..., Allocation..., PreviewInvoice...,
>>>>> PreviewCredit... and GLEntries...) have more work than only format the
>>>>> string between the table-datacell tags ("<td></td>"),  I "feel" that
>>>>> the suffix "FormatString" can conduct to an error. Comments? Suggestions?
>>>>>
>>>> Can't these variables be made to have the correct number of cells
>>>> ("<td></td>") ? My impression is that this is all that is required to
>>>> fix this script?
>>>>
>>>>> 5.2. "TD..." suffix. We need to show evident the quantity of columns
>>>>> in a table-row (pairs of "<td></td>" inside a "<tr></tr>"). I am not
>>>>> happy adding "_TD..." suffix, but I can not think of another idea.
>>>>> Comments? Suggestions?
>>>> I wasn't keen on the TD suffixes and initially didn't get what they
>>>> were. Better off just to make the variables have the correct number of
>>>> cells.
>>>>> 5.3. Any suggestion to  have a code easy to understand for a newcomer.
>>>>>
>>>> Follow
>>>>
>>>> http://www.weberp.org/CodingConventions.html
>>>>
>>>> rigorously. Of course this is just my take on this and the way much of
>>>> the code is laid out as far as possible. If some of it isn't then
>>>> changes to bring it in line are most welcome.
>>>>
>>>> Every so often someone comes up with a bright idea to change the way the
>>>> code is.
>>>>
>>>> I am not interested re-writing the code a different way.
>>>>
>>>> I am interested in bug fixes and new functionality.
>>>>
>>>> However, I took Andrew's point that abstracting some code to functions
>>>> would make some changes easier and use less characters and there is a
>>>> middle ground with abstraction. I think we have a reasonable compromise
>>>> - and I make no apology for erring on the side of readability - it is a
>>>> core goal.
>>>>
>>>> Phil