Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Merge formatters into matchers #677

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

tienvx
Copy link
Contributor

@tienvx tienvx commented Oct 9, 2024

Apply SOLID

L - Liskov Substitution Principle

Currently most of formatter's code are like this:

class BooleanFormatter implements FormatterInterface
{
    public function format(MatcherInterface $matcher): string
    {
        if (!$matcher instanceof Boolean) {
            throw MatcherNotSupported();
        }

        return 'formatted value';
    }
}

Now it's just a method inside matcher:

class BooleanMatcher
{
    public function formatExpression(): string
    {
         return 'formatted value';
    }
}

I - Interface Segregation Principle

Create bunch of small interfaces. Matchers can decide to implement interface that it need:

class Boolean implements JsonFormattableInterface, ExpressionFormattableInterface
{
    public function formatJson(): array {}
    public function formatExpression(): string {}
}
class StatusCode implements JsonFormattableInterface
{
    public function formatJson(): array {}
}

no need to implement a big interface then throw exception on unused method like before:

class ArrayContains
{
    public function createExpressionFormatter(): ExpressionFormatterInterface
    {
        throw new MatcherNotSupportedException("ArrayContains matcher doesn't support expression formatter");
    }
}

O - Open-Closed Principle

Instead of using array and string, wrap them in new classes Attributes and Expression

Simplify generator flow

Old flow

old

New flow

new

Breaking changes

I tried not to introduce any breaking changes. All matchers can be used normally using the helper class Matcher

@coveralls
Copy link

coveralls commented Oct 9, 2024

Pull Request Test Coverage Report for Build 11248263126

Details

  • 516 of 516 (100.0%) changed or added relevant lines in 53 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 96.371%

Totals Coverage Status
Change from base Build 11173981158: 0.5%
Covered Lines: 2443
Relevant Lines: 2535

💛 - Coveralls

@tienvx tienvx requested review from JP-Ellis and YOU54F October 9, 2024 05:18
@tienvx tienvx marked this pull request as ready for review October 9, 2024 05:18
@tienvx tienvx merged commit 9ab2ac9 into pact-foundation:master Oct 14, 2024
18 checks passed
@tienvx tienvx deleted the refactor-matchers branch October 14, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants