mesnitu / EasyPopulate4BookX

Data import and export module for Zencart 1.3.x and 1.5.x
GNU General Public License v2.0
1 stars 0 forks source link

products_id not "transferred" to observed import action #10

Closed mc12345678 closed 8 years ago

mc12345678 commented 8 years ago

Through discussions and planning, I had identified a point in the import program code that seemed like a satisfactory observer location to process the data read from the file and to be or possibly be processed by bookX (or any product type). As a result of the chosen point, and subsequent code, it appears that the $products_id data is not populated at this point in the observer.

Looking at the observer section, the following variables are made global: $v_products_id, $v_products_model, $v_products_name, all of these variables can be determined from the data that is contained in $items and using $filelayout, ie. The code in EP4 that selects/sets those variables can be used inside the bookx code. Part of the reason of previously suggesting the notifier that has been suggested was so that the bookx code could independently determine/modify those three pieces of data or others generated from the file read code without affecting downstream operations and expect to not be bothered by upstream operations either... By passing these three variables as global, the bookx code is completely dependent on their value when trying to process... This is not a good choice and could lead to unexpected results. The data read from the file should not in anyway be modified until it is to be read again and I would suggest using that data whenever "referencing" the current data line.

Based on the above is why a notifier was not placed "at the end" of the while loop... There are too many dependencies on the previous data being correct for the desired/intended usage and further understand that by placing it at the end in this way that every bookx file becomes processed through all of the code, instead of possibly being sped up by preventing "unnecessary" code from being executed to parse the file data... I just don't yet see the benefit of the notifier being at the end like it is requested when it can be handled as necessary with the existing notifier(s).

mesnitu commented 8 years ago

ok, so the classic situation where one must really know what is doing. I'm afraid it's not the case. The last changes I've made, try to enter before the EP4_START, and all that, was based on not knowing to fully understand the class. I understand the base, but not entirely know how to take advantage of it. Probably all that I've done, could be done in that class. And I didn't test all possibilities, but I'm running out of time. I didn't even test if the bookx is inside the EP4 filtres. If it's not, it's not a good situation.

So, if this is not good, It's better to probably change it, but it will take some time. for me aquire more Knowledge on the matter

mc12345678 commented 8 years ago

Seems like this item/issue might be able to be closed?

mesnitu commented 8 years ago

Yep. All good