Code:1) BAD MARKUP 1.1) INVALID - Your markup is broken, and will be rendered in different ways depending on how the browser decides to error-correct it. The validation tool at http://validator.w3.org/ can probably help you fix it (although note that the validator is just a tool, and not authoritative). http://validator.w3.org/docs/why.html http://validator.w3.org/ 1.2) OUTDATED DOCTYPE - Your page is using an outdated DTD. Probably, this means a Transitional DOCTYPE or one of the older HTML3 DOCTYPEs. You should update your DOCTYPE and revalidate your page as one of the latest Strict DOCTYPEs. 1.3) UNSUPPORTED MARKUP - Your page is using a DTD or markup language that is unsupported by a large percentage of users. If you are running the site in a controlled environment, this may be fine, but as a general-purpose web page, you should migrate to a better-supported language/DTD. If your page appears to work in all major browsers, this probably means that you are sending an incorrect Content-Type header and having it parsed by browsers as another markup language, and error-corrected into validity. At the time of writing, this applies to HTML/XHTML: people commonly choose an XHTML DTD without realising that what browsers are parsing is actually INVALID HTML (see 1.1). The validator does not currently choose a parsing mode in the same manner as major browsers. http://www.webdevout.net/articles/beware-of-xhtml http://www.hixie.ch/advocacy/xhtml 1.4) PRESENTATIONAL MARKUP - Your page uses obsolete markup designed for the era before CSS that is bloating your pages and making maintenance difficult. You should be using CSS instead. This also applies to non-presentational elements being used for presentational purposes (such as tables being used to structure layout rather than to display tabular data). http://www.w3.org/2001/tag/doc/contentPresentation-26.html http://www.hotdesign.com/seybold/ 2) STYLESHEETS 2.1) PIXEL SIZES - You have elements sized in pixels (px), which is highly discouraged for anything but images. It will cause one's page to become inflexible and thereby inaccessible, since the user will be unable to adapt the page to his/her needs. Pixels are absolute sizes on the screen, and their real-life size may differ drastically between resolutions. 2.2) EM FONT SIZES - You have specified one or more of your font sizes in ems (em). While ems may seem the ideal unit for specifying font sizes, they have poor support in some versions of Internet Explorer. Percentages should be used for sizing fonts instead. 3) CLIENT-SIDE PROGRAMMING 3.1) INNERHTML - You have used the .innerHTML property in your code. innerHTML is non-standard, results in bad code, and should be avoided wherever possible. There are a few rare cases where it is necessary to use it or an equivalent; this is not one of them. There are a variety of problems caused or aggravated by the use of innerHTML, aside from it being invalid: to pick a sample, it requires representing code as strings and demands proper escaping (see 4.1 and 5.1); it does not always behave as expected, for example destroying unserialisable properties of DOM nodes such as event handlers or unexpectedly closing elements; and it destroys and recreates *all* nodes under the *parent* of the node one modifies. http://www.grauw.nl/blog/entry/475 http://slayeroffice.com/articles/innerHTML_alternatives/#intro http://developer.mozilla.org/en/Gecko_DOM_Reference 3.2) UNINTENTIONAL GLOBALS - It seems as though you have omitted a `var` keyword. In ECMAScript, variables are declared and initialised in the form `var <identifier> [= <value>];`. As a result of omitting that keyword, what you probably thought was a local variable becomes a property of the global object, which is both bad coding style (see 4.2) and allows other code to accidentally overwrite its value, potentially introducing bugs which are very difficult to track down. 3.3) MISSING SEMICOLONS - Your code is missing semicolons after statements. ECMAScript has a feature called 'automatic semicolon insertion' in which semicolons are automatically inserted at line breaks in what its inventors considered to be useful positions, and as a result semicolons can be dropped from the ends of many statements in ECMAScript with no obvious harm. However, the rules used to decide where statements should and should not end are unintuitive, and can lead to peculiar errors if the interpreter takes what one intended to be two separate statements as one. Additionally, it harms readability to have to work out where each statement breaks. Always insert a semicolon at the end of each statement (but they are not necessary after blocks, unless said block is part of a statement). http://www.howtocreate.co.uk/tutorials/javascript/semicolons 4) SERVER-SIDE PROGRAMMING 4.1) MIXING CODE AND OUTPUT - You have executable code mixed into your output, or vice-versa. Some server-side languages encourage mixing server-side code and the markup it outputs. This leads to code which is both difficult to read and difficult to maintain. Storing markup in strings is inefficient and requires meticulous escaping of special characters. It is advisable to break out of the parsing mode for your server-side language when outputting markup, and to execute as much code as possible before ever beginning output, dropping back into it only the bare minimum necessary for flow control and such. When working on large projects, use of a template engine and/or an ORM can aid this endeavour significantly. http://particletree.com/features/4-layers-of-separation/ http://www.smarty.net/ http://genshi.edgewall.org/ http://propel.phpdb.org/ http://www.doctrine-project.org/ http://www.sqlalchemy.org/ 4.2) RELATIVE REDIRECTS - You have redirected using an HTTP 3xx REDIRECT status code, but sent a relative URL in the Location header field. This is prohibited by the HTTP specification: Location URIs must be absolute. It is very easy to convert a relative URL to an absolute one: simply prepend the basename of the request URI. 5) GENERAL PROGRAMMING 5.1) REPRESENTING CODE AS STRINGS - You have represented some form of code as strings. Representing executable code as strings is *always* bad style, and is never necessary in any language worth its salt. Strings are a datatype, and one that is not capable of cleanly representing code. Storing code in strings is messy, inefficient, and, if mishandled, insecure. If you're using eval(), innerHTML and the like, you're almost certainly doing it wrong. There are exceptions: PHP requires any anonymous functions to be represented as strings, making it a necessary evil, and JSON, whilst treated by the runtime as executable code, is actually a form of data. http://blogs.msdn.com/ericlippert/archive/2003/11/01/53329.aspx 5.2) EXCESSIVE USE OF GLOBALS - You have used far too many globals. Collisions in the global namespace are very likely, and introduce bugs that are hard to track down, especially in dynamically-typed languages. Your application should ideally contain only one global: a namespace containing all the other globals. This is unfortunately not possible in all languages, but one should endeavour to limit one's globals to as few as possible nevertheless. If your language supports it, closures or classes are almost always a viable alternative to globals. http://c5.com/cgi/wiki?GlobalVariablesAreBad 5.3) PREMATURE SPECIALISATION - Your code has functions that do far more than their fair share of work, and as a result are not at all reusable (and also have a negative effect upon program structure). A program is best constructed starting with the most general functions and datatypes possible, and building them up into ones more specific to the problem at hand. This optimises code reuse and decoupling. Particularly general functions and types may be worth storing in a separate library that one can reuse in each new application that requires them. It's better by far to have many small functions than a few big functions; if one's function is bigger than perhaps ten lines, one should seriously consider breaking it down. Function-calling overhead is negligible for all but the most resource-intensive applications. Rendering the output displayed to the user should be the last layer of specialisation, done only after computing all the data necessary to do so. 5.4) FAILURE TO ESCAPE - You have forgotten to escape some string(s) within your code. Sometimes, despite our best efforts, it is necessary to work with a string of code (such as an SQL query) or to embed data where it may be interpreted as code if malformed (such as an HTML document or a JSON object). In these cases, if the data is coming from an untrusted source (such as a user) it is *vital* to escape those strings appropriately. Failure to do so could lead to a security hole that will result in one's machine or the machines or information of one's clients being compromised. 5.5) TIGHT COUPLING - Your program is very tightly coupled. The functions and datatypes are not very reusable, since they all rely upon one another. This is often a result of 4.1, 5.2 or 5.3, but can also arise from various types of code being mixed together -- markup, style, and script, for example. The program probably does or will contain duplicated, copy-and-pasted code.
Bookmarks