Skip to content

Things i came across

kanduvisla edited this page Apr 23, 2012 · 3 revisions

possible bugs and improvements

  • Why are table associations stored as hide_association but sent as a parameter called show_association? Seems like a bit of Klingon logic to me. I saved show_association in the Section XML-files to be consistent, but also added the hide_association-parameter for backward compatibility in the fetchAssociatedSections()-function.
  • In FieldManager::fetchByXPath() I also added $data['field_id'], which is the same as $data['id'], but I noticed that some fields work with field_id instead of id. So I put it there for backward compatibility.
  • I noticed that in the FieldManager the $field->setArray()-parameter is called twice: once for storing the data that came out of tbl_fields, and once for data that came out of tbl_fields_[xxx]. This would mean that if the seconde table had a similar fieldname (element_name for example), this would overwrite the first set parameter. This could cause unexpected behavior and is in my eyes considered a bug. I'm thinking about putting the options in a separate <options />-element in the field area of the Section XML, but this would mean that all fields should differ between $this->get() and a new $this->getOption() to get their options (although this could also be made backward compatible).
  • The pages- and section editor are currently still focused on storing their information in the database. Therefore there are many calls to the managers to store page types and section fields for example, which result in numerous write-and-reindex-operations to the disk. This should be optimized in the new workflow to minimize the write-and-reindex-operations.
  • A lot of code is not very clear about it's function. Variables like $missing_cfs, $_vals, $s without any explanation occur a lot. Also function with weird names like __doit() with no documentation whatsoever. Also inline comments are rarely used. Some functions are hundreds of line of code with only a few inline comments to explain what's happening. It should be good if Symphony would follow a code convention where a single programming style is maintained and everything is thouroughly documented.
  • I found some variables which aren't used in their function. These could also be cleaned up.

a second thought on lookup tables

  • Pages have a lookup table, but in my opinion this could be omitted. It is there now for backward compatibility with fields that used tbl_pages to keep working with page ID's, but that's also the only reason. A better approach would be to adjust these fields (there aren't a lot) and work only with unique hashes for pages.
  • Sections and unique hashes, same story as pages. It's there for backward compatibility so extensions can work with section ID's, but this could theoretically also work with just using unique hashes.
  • Fields and unique hashes, could also be omitted, but for cosmetic reasons it might be better to keep this one. Otherwise you would get data table names like tbl_entries_data_60f6ac47455b6dc8c89ad499cc932ec5.

So... theoretically all lookup tables could even be dropped!