Skip to content
Permalink
Branch: master
Commits on May 27, 2018
  1. Merge pull request #5 from hellofromtonya/torque/code-review/part4

    hellofromtonya committed May 27, 2018
    Making FilterWPQuery Reusable, Swappable, & Flexible
  2. Initialized the implementation for the integration tests.

    hellofromtonya committed May 27, 2018
    Before we can go get the posts, first we need to initialize FilterWPQuery by
    loading the implementation.
  3. Fixed PostsGenerator imports. Removed `static` from method.

    hellofromtonya committed May 27, 2018
    1. Whoops, forgot to import the classes.
    2. The private method is on the object and not static.
  4. Removed WP_Post mock autoload & downgraded PHPUnit to 5.7.9.

    hellofromtonya committed May 27, 2018
    WP_Post mock begin autoloaded can cause the integration tests to fail.  Load it only if needed.
    
    With the latest version of PHPUnit, a fatal error is thrown in WP Core Test Suite:
    
    Class PHPUnit_Util_Test may not inherit from final class (PHPUnit\Util\Test) in /tmp/wordpress-tests-lib/includes/phpunit6-compat.php on line 18
    
    Downgraded to version 5.7.9 to resolve the issue.
  5. Mocked in unit tests.

    hellofromtonya committed May 27, 2018
    The new FilterWPQuery requires an implementation for getting the posts.
    This commit mocks the interface and then loads it via the `init()` method.
    
    In addition, switched to using Mockery and Brain Monkey instead of a separate
    mocked stub.
  6. Added Brain Monkey & Mockery to the test suites.

    hellofromtonya committed May 27, 2018
    We'll use Brain Monkey in the unit tests.
    
    Mockery we'll use in both test suites.
Commits on May 26, 2018
  1. Merge pull request #4 from hellofromtonya/torque/code-review/part3

    Shelob9 committed May 26, 2018
    Code Review & Refactoring for Posts Generator
  2. Merge pull request #3 from hellofromtonya/torque/code-review/part2

    Shelob9 committed May 26, 2018
    Code Review & Refactoring for FilterWPQuery
Commits on May 11, 2018
  1. A little clean up.

    hellofromtonya committed May 11, 2018
  2. Injected the implementation during plugin launch.

    hellofromtonya committed May 11, 2018
    During the launch, we initialize the FilterWPQuery by injecting
    which implementation we want for our project.
  3. Added an init() method & injected the implementation.

    hellofromtonya committed May 11, 2018
    Notice that we are type hinting the injected object using the
    contract.  This contract serves an interface between the FilterWPQuery
    and each implementation.
    
    Using this design, the FilterWPQuery class does not care which
    implementation is wired to it.  Rather, it serves as a generic object.
    But because of the contract, the FilterWPQuery knows how to interface
    with it.
  4. Wired the generic object to getPosts.

    hellofromtonya committed May 11, 2018
    Notice how getPosts() uses the new generic $contentGetter object
    to invoke through the contract to the specific implementation that
    is bound to the $contentGetter property.
  5. Added the posts generator implementation.

    hellofromtonya committed May 11, 2018
    Notice that this implementation implements the ContentGetterContract.
    Therefore, it has the standard `getContent` method, which will be
    invoked by FilterWPQuery.
  6. Added "getting the content" interface.

    hellofromtonya committed May 11, 2018
    This interface is a contract that strictly defines how to interact
    with each "getting the content" implementation.
    
    We can use this contract for type hinting when we inject an
    implementation into FilterWPQuery.
Commits on Apr 26, 2018
  1. adfjlajf

    hellofromtonya committed Apr 26, 2018
Commits on Apr 25, 2018
  1. Defined the embedded variable.

    hellofromtonya committed Apr 25, 2018
    PHP needs your help to quickly identify the variable that is
    embedded inside of a string.  Wrapping the variable inside of
    curly braces tells the PHP parser: "Hey, this is an embedded
    variable."
    
    Why?
    
    Imagine that the title had another character after the variable.
    How would PHP know that the variable is `$postNumber` and not that
    plus the other character(s).
    
    The parser is greedy.  And a string is comprised of different
    characters.  Where does a variable start and end?
    
    The better practice of wrapping an embedded variable in curly
    braces is expressive and declarative.
    
    As an added benefit, it helps you and me to quickly identify
    it as variable to understand that its value will be swapped out
    and placed in the post's new title.
    
    @see http://php.net/manual/en/language.types.string.php#language.types.string.parsing
  2. Renamed $i to tell us what value it represents.

    hellofromtonya committed Apr 25, 2018
    A name matters. Why? It tells us what's going on and why.  For
    a variable, its name tells you what value it represents.
    
    That simple step of changing `$i` to `$postNumber` tells you
    that the value is the post's number.  You more quickly get
    what's going on.
  3. Removed the unnecessary array index.

    hellofromtonya committed Apr 25, 2018
    The posts start at index 0 and so does an indexed array.
    Therefore, there is no need to specifically specify the new
    post's position in the array.
    
    This change simplifies the code and makes it more readable.
    But it also makes it slightly more performant as PHP does not
    have to lookup the value presented by variable.
  4. Aligned assignments.

    hellofromtonya committed Apr 25, 2018
    Formatting matters.  It aids in our ability to jump into the code
    and figure out what's going on...quickly.
    
    By aligning a group of assignments, our eyes are draw in to notice
    that each of the lines of code within the loop are assignment
    operations.
  5. Imported classes.

    hellofromtonya committed Apr 25, 2018
    Backslashes add more characters for us to read and clutter up
    the code.  A better approach is to import the classes into
    the current namespace via `use`.  By doing this, we can
    use the class without the preceding backslash.
  6. Removed unnecessary parentheses from post instantiation.

    hellofromtonya committed Apr 25, 2018
    The keyword `new` creates an object for us.  The only time we
    need to wrap an object inside of a pair of parentheses is when
    we want to chain a method directly to the object without assigning
    the object to a variable.
    
    Here, removing the unnecessary () makes the code more readable.
    
    Remember, formatting matters. :)
  7. Abstract posts generation to generator method.

    hellofromtonya committed Apr 25, 2018
    The getPosts() method's intent to fetch the posts for the
    pre-query. In a real world plugin, it's job is to be a
    handler or manager.  It decides which mechanism to use to
    either get or generate posts.
    
    That means the job of dynamically generating posts is a
    different task.
    
    The inline comments are there as a neon light telling us
    to refactor.  By abstracting the post generator to its
    own method, we can:
    
    1. get rid of the inline comments
    2. set up the class to be a handler or manager, meaning that
    we can extend the functionality to load different implementations
    based upon our business rules and given inputs.
  8. Made code style consistent.

    hellofromtonya committed Apr 25, 2018
    Consistency in the code is a vital component of code quality
    and readability.
    
    1. Format of the return type declaration.
    2. One empty line before the namespace.
    3. One empty line between methods.
    4. No empty line between class opening curly brace and first element.
    
    Be Consistent.
  9. Fixed tests per our refactoring process.

    hellofromtonya committed Apr 25, 2018
    Now that we've refactored the FilterWPQuery class and its
    interface, the test suites need to be updated too.
    
    1. Rename the callback method.
    2. Pass in the postsOrNull input to shouldFilter.
  10. Replaced hard-coded priority with the property.

    hellofromtonya committed Apr 25, 2018
    The class defines the priority number.  It's redundant to
    hard-code the priority number in the `add_filter` and `remove_filter`
    tasks.  This change makes the code DRY.
    
    What are the advantages here?
    
    The priority number can change in one place, i.e. in the property.
    This is important as you may want to change the priority.  But you
    might forget to change it in each of the instances. Whoopsie.  That
    will create a bug.
    
    Using the new strategy, the priority number is set in only one place.
    Then the code gets that value when doing its work.
  11. Renamed "callback" to tell us what its intent.

    hellofromtonya committed Apr 24, 2018
    The word "callback" is generic.  It does tell us that it is
    bound to something that will invoke it.  But it does not
    tell us what it will do, i.e. what its behavior is when it's
    called.
    
    This "callback" job is to get the posts for a RESTful request.
    It's bound to a filter, i.e. `post_pre_query`, which is fired
    _before_ the query takes place.
    
    When figuring out a name, we first identify what it does:
    
    1. It's a filter, which changes the input for 'post_pre_query' event.
    2. It gets or generates an array of posts to be used for the query.
    
    Using a name such as `filterPreQuery` tells us that when this
    method is called, it will "filter the pre query.
Commits on Apr 24, 2018
  1. Skip the assignment and just return.

    hellofromtonya committed Apr 24, 2018
    The assignment variable is unnecessary, as we are not using the
    variable other than to return the value it represents.  Making
    an unnecessary assignment makes the code less performant.  Why?
    
    1. PHP has to create the new variable in its symbols table.
    2. It has to bind that spot to the value's memory location.
    3. Then it shifts control to the next line.
    4. Next, it has to lookup the value for that variable before
    returning.
    
    These extra steps add memory and increase the code's execution
    time.
    
    By refactoring to just return, we eliminate these extra steps.
    The code will run faster.  But it is also more readable, making it
    more maintainable.
  2. Just return the conditional's decision.

    hellofromtonya committed Apr 24, 2018
    The REST checker makes it decision and then evaluates into a
    boolean.  There is no need for us to explicitly return false or
    true.  Instead, a better strategy is to return the conditional's
    decision.
    
    Why?
    
    1. It's less code to read and maintain.
    2. It's more performant (faster) because there is only one line
    of code for PHP to process.
  3. Abstracted the REST checker to improve readability.

    hellofromtonya committed Apr 24, 2018
    Step 3 is create a new private method for the REST checker.
    
    Why?
    
    Without the inline comment, the conditional's intent is not clear.
    did_action('rest_api_init') does not tell us what this checker is
    doing.  It just tells us that the 'rest_api_init' event fired.
    
    Our goal is to build our code to be highly human readable, such
    that anyone (including you) can quickly understand what's going on.
    We design code to _tell us_ what's happening and why.
    
    By moving this function to a new method, we use the name of the
    method to tell us it's intent, i.e. it's checking if we are
    doing a REST request.
  4. Flip the checkers order to make more performant.

    hellofromtonya committed Apr 24, 2018
    Step 2 is to flip the checkers order.  The result improves the
    performance of the code _when_ the given input is not null.
    
    Why?
    
    The PHP is_null function is faster than running did_action().
  5. Move the Null Checker.

    hellofromtonya committed Apr 24, 2018
    Our goal is to convert the shouldFilter() method into a
    decision maker to determine if the code should filter or not.
    
    Step 1 is to move the null checker into this method and pass the
    input to it.
    
    By doing this step, we can remove the inline comments and rely
    on the shouldFilter() method to be the 'return early' decision
    maker.
  6. Reorganized callback into "return early" strategy.

    hellofromtonya committed Apr 24, 2018
    Using the strategy for Part 1 of the series, the callback
    method is reorganized.  The guard clauses are clearly
    identified and will bail out if a "no go" condition is found.
    
    @see https://torquemag.io/2018/04/code-review-part-1-fixing-design-flaw-return-early-strategy/
Commits on Mar 31, 2018
  1. Tidy composer.json

    Shelob9 committed Mar 31, 2018
  2. code coverage with codecov

    Shelob9 committed Mar 31, 2018
Older
You can’t perform that action at this time.