Skip to content

Makes FakeRepository from Ch5 more like one from Ch2 - #355

Merged
hjwp merged 1 commit into
cosmicpython:masterfrom
filiplajszczak:abc-consistency
May 12, 2025
Merged

Makes FakeRepository from Ch5 more like one from Ch2#355
hjwp merged 1 commit into
cosmicpython:masterfrom
filiplajszczak:abc-consistency

Conversation

@filiplajszczak

Copy link
Copy Markdown
Contributor

It looks like FakeRepository in Chapter 5 introduced in 50bfa5c was left in original state, while in the meantime ABC was introduced into chapter 2 in 1795366, so here we make it match again.

As a side note, the alternative approach would be to use classmethod instead of staticmethod but it depends very much on personal preferences. It's not what I propose in that PR, but it could look like that:

class FakeRepository(repository.AbstractRepository):
    @classmethod
    def for_batch(cls, ref, sku, qty, eta=None):
        return cls([model.Batch(ref, sku, qty, eta)])

…stent with the current shape of it in Chapter 2.
@hjwp
hjwp merged commit 432b791 into cosmicpython:master May 12, 2025
@hjwp

hjwp commented May 12, 2025

Copy link
Copy Markdown
Contributor

oo nice catch thanks Filip!

am always on the fence about classmethods! i find the cls(...) less readable but otoh, extensibility, so... yep, on the fence.

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.

2 participants