#General Feedback ##Large Methods Some of the class methods are really large - e.g. `ChessGame ::get_legal_moves_list()` which consumes ~150 lines. There is a lot of redundancy - especially in the code to assign `$directions_list`. That code should be moved out to a separate method. There are 4 calls to `::add_slide_and_slidecapture_moves_to_moves_list()` and those could all likely be simplified to a single call whenever the piece type is `bishop`, `rook`, `queen` or `king`. Generally whenever a method goes beyone ~10-15 lines you should consider refactoring it. In addition to readability it will also help with testing. For more on cleaning up code, check out [Rafael Dohms talk about cleaning up code](https://www.slideshare.net/rdohms/bettercode-phpbenelux212alternate/18-public_function_createPostrequest_entity_new) (or [watch a presentation he did of it, hosted on youtube](https://www.youtube.com/watch?v=GtB5DAfOWMQ&t=2910s)). ##Exception Handling Exceptions are thrown but never caught. Currently if an exception is thrown, that is displayed to the user, like the one I see when I change the URL of a move: > **Fatal error**: Uncaught exception 'Exception' with message 'ChessBoard Class - Invalid FEN' in /home/clania/clania.net/admiraladama/chess_v2/ChessBoard.php:54 Stack trace: #0 /home/clania/clania.net/admiraladama/chess_v2/index.php(23): ChessBoard->set_fen('rnbqkbnr/pppp1p...') #1 {main} thrown in **/home/clania/clania.net/admiraladama/chess_v2/ChessBoard.php** on line **54** Ideally those would be handled - perhaps in Index.php with a `try`/`catch` that may set the value of an error variable and the view would display that error message instead of the board. ###PHP mixed within HTML Generally it is best to separate the business logic (PHP) from the view (HTML). Using a template engine could help for this aspect - e.g. [Smarty](https://www.smarty.net/), [Twig](https://twig.symfony.com/), etc. For more information on this topic, prefer to [this article](http://thisinterestsme.com/mixing-php-html/). #More Specific points ##PHP ###Declaring Instance/Member variables The class definitions appear to use the PHP 4 style declarations for instance variables (i.e. `var`). Note that while support of this syntax isn't currently deprecated, it may be in the future: > **Note**: The PHP 4 method of declaring a variable with the `var` keyword is still supported for compatibility reasons (as a synonym for the public keyword). In PHP 5 before 5.1.3, its usage would generate an **`E_STRICT`** warning.<sup>[1](http://php.net/manual/en/language.oop5.visibility.php#language.oop5.basic.class.class.name)</sup> ###Getting the timestamp with microseconds Index.php starts with the following lines: > $time = microtime(); $time = explode(' ', $time); $time = $time[1] + $time[0]; $start = $time; Some would argue that readability suffers here because on the first line, $time is a string This appears to function identically to the [Example 1: Example #1 Timing script execution with microtime()](http://php.net/manual/en/function.microtime.php#example-2812) on the PHP Documentation for [`microtime()`](http://php.net/manual/en/function.microtime.php). Notice that the function in that example contains the comment: > /** * Simple function to replicate PHP 5 behaviour */ And notice [the next example](http://php.net/manual/en/function.microtime.php#example-2813) is titled **Example #2 Timing script execution in PHP 5**, where it simply calls `microtime()` with `true` for the parameter [`get_as_float`](http://php.net/manual/en/function.microtime.php#refsect1-function.microtime-parameters). So instead of using those first three lines of Index.php could simply be replaced with the call to `microtime(true)` for the same functionality. $start = microtime(true); ###Error Reporting Index.php also contains the following line: error_reporting(-1); According to the code in [**Example #1 error_reporting() examples**](http://php.net/manual/en/function.error-reporting.php#example-494) in the documentation for [`error_reporting()`](http://php.net/manual/en/function.error-reporting.php#example-494), [`E_ALL`](http://php.net/manual/en/errorfunc.constants.php#errorfunc.constants.errorlevels.e-all) can also be used for that: > // Report all PHP errors (see changelog) error_reporting(E_ALL); > // Report all PHP errors error_reporting(-1); Using the constant like that (i.e. `error_reporting(E_ALL);`) is more readable for anyone who doesn't remember what `-1` signifies when passed to that ##CSS ###Simplification of `padding` for status box The four padding styles can be simplified from: > padding-top: 2px; padding-bottom: 2px; padding-left: 4px; padding-right: 4px; To the following: padding: 2px 4px; Because: > When two values are specified, the first padding applies to the **top and bottom**, the second to the **left and right**. <sup>[2](https://developer.mozilla.org/en-US/docs/Web/CSS/padding#Syntax)</sup> <sup>1</sup><sub>http://php.net/manual/en/language.oop5.visibility.php#language.oop5.basic.class.class.name</sub> <sup>2</sup><sub>https://developer.mozilla.org/en-US/docs/Web/CSS/padding#Syntax</sub>