Posting in the Magento forums has been disabled pending the implementation of a new and improved forum solution which should better serve the community.

For new questions please post at magento.stackexchange.com, the community-run support site for the Magento community. We will be providing updates on the new forum solution soon. For questions or concerns please email community@magento.com.

Magento Forum

Seven Year Veteran’s thoughts after six weeks
 
Aki_Tendo
Jr. Member
 
Total Posts:  3
Joined:  2012-04-12
 

Hello.  I recently was hired into a company which is converting their custom in house OS commerce package to Magento Enterprise. I’ve been working with PHP for seven years, and have experience with several of Magento’s spiritual ancestors / competitors including ZenCart and OSCommerce.  These are my thoughts after six weeks.

Overall code organization is above average, but still leaves some things to be desired.  The XML configurations for different object types is inconsistent - and the XML parser doesn’t throw an error on most failure types leading to some degree of frustration.  The most serious inconsistency I’ve found is controller overrides - when you do one of these you have to use a require_once call on the router you are extending from.

There is still considerable code duplication, particularly in the templates, and drastically in the XML files.  In my opinion, the XML has exceeded the scope of being the configuration of the store and become a programming language in its own right. This problem is not exclusive to Magento - but a common one of deferment. Eventually programmatic decisions must be made.

Magento classes are not granular enough to allow for effective polymorphic programming. Most all functions I’ve seen in the system perform multiple tasks. 

Some specifics.  The template hinting system is atrocious and in IE it is non-functional because it forces the browser into quirks mode because the first of the hints given will occur before the <!DOCTYPE HTML> mark.  Trying to work on an html 5 marked up page when the browser is using the IE 5.5 rendering engine is migrane inducing. Aside from this issue, the visible bordering at each template start and stop is ill conceived and prevents the page from displaying as intended.  My first change to the code (applied using the ZenCart style file override system) was on fetchView

if ($this->getShowTemplateHints()) {
         
/* the original code is here in my file, omitted here for brevity */
            
}
        } 
else if (isset($_SERVER['MAGE_IS_DEVELOPER_MODE'])) {
            
/*
             * Doing this before the <!DOCTYPE html> call throws IE into quirks mode.
             * Quirks mode bad.
             */
            
if (substr(dirname($fileName), -4) != 'page'{
                
echo "<!-- BEGIN TEMPLATE: {$fileName} using block ".get_class($this)." -->";
            
}
        }

So we don’t declare the page template in use (rarely useful to know) and the information concerning blocks and templates is placed into comments.  In Google’s Chrome browser you can then right click an element, click inspect element and in the console look for the begin template call immediately preceding it to see where it came from.  As a bonus, the page layout is NOT disturbed by this.  Since knowing where the template ends is useful this follows

if ($this->getShowTemplateHints()) {
            
echo '</div>';
        
else if (isset($_SERVER['MAGE_IS_DEVELOPER_MODE'])) {
            
/*
             * Doing this before the <!DOCTYPE html> call throws IE into quirks mode.
             * Quirks mode bad.
             */
            
if (substr(dirname($fileName), -4) != 'page'{
                
echo "<!-- END TEMPLATE: {$fileName} using block ".get_class($this)." -->";
            
}
        }

The Javascript code of Magento reads like the sort of JS you’d expect from PHP programmers doing JS because they have to, not because they want to.  Rare indeed is the moment when the Magento JS takes advantage of either JS itself or prototype.  Also, loading jQuery adds needless overhead and client-side code bloat.  Given the nature of the tasks Magento has to do jQuery should probably win out (I say this despite liking prototype.js more).

A repeated problem in the templates is the use of onEvent handlers.  Guys, those are 1998 conventions.  MVC on the client side means Javascript control - HTML data modelling and CSS view specification.  Inline JS should be avoided, especially event handlers. This sort of code is brittle, hard to test and hard to code for.

Speaking of the CSS - it’s a nightmare.  I recently reviewed the whole sheet and converted it to LESS.  There are redundant declarations, broken statements abound, and !important calls everwhere.  I’m of the extreme opinion that !important should never need to be used in a CSS layout - it shows the writer of the file doesn’t understand the cascade rules and it creates a headache for those of us that come after that do.

Enough with the negative.  As I said the code organization is above average - and despite the size of the base it’s relatively easy to find things.  As critical as I am of the deluge of XML files I cannot call a better solution to mind easily. The code is beautifully if not consistently formats - it looks like have the team uses open bracing and the other inline bracing. But the indents are accurate and I’ve yet to hit anything obnoxiously bad.  Things are getting better, and it’s certainly more powerful than the things that came before ( ZenCart and ::shudders:: OSCommerce )

 
Magento Community Magento Community
Magento Community
Magento Community
 
Ryan Grow
Jr. Member
 
Total Posts:  21
Joined:  2009-12-04
 

Is there a way to “Thumbs Up” or +1 this post, you are so right on, I seem to say this stuff every time I have to do something in Magento.

But I just wanted to throw my two cents in on your XML comment;

Now I totally get what they are trying to do by using XML for configuration, but they really missed the mark on it.
Instead of an easy to read and follow file, it has become a huge frustrating, mess and a language all it’s own. The only way to know what you are doing in it is to either read the PHP code that processes the XML or have to find an have the skimpy documentation open and then you still have to make guesses.

Wouldn’t it just be easier and faster to have it all stored in a PHP array? I mean isn’t that what it becomes anyways? It’s just that I have noticed that PHP can be slow at converting XML into something useful and I could see this as really speeding things up. But then again I guess it would be hard to remove since it would break their whole system cache models because that stores all those files as XML…

 
Magento Community Magento Community
Magento Community
Magento Community
Magento Community
Magento Community
Back to top