Skip to content
This repository has been archived by the owner on Jul 17, 2020. It is now read-only.

[WIP] Pimple, Interfaces #5

Merged
merged 1 commit into from
Jul 6, 2013
Merged

[WIP] Pimple, Interfaces #5

merged 1 commit into from
Jul 6, 2013

Conversation

liuggio
Copy link
Contributor

@liuggio liuggio commented Jun 25, 2013

  • Gather feedback about what do to with \Pimple.
  • move all the examples and 'Simple*' classes into a 'example' folder.
  • Gather feedback about the objective of the ObjectLoaderFactory.
  • Improve CS.

I didn't change so much, I added an Abstract Repository, a simple Associator for class->worker, and few Interfaces

  • IdentityMapSynchronizerInterface
  • ObjectRepositoryInterface
  • IdentityExtractorFactoryInterface
  • ObjectLoaderFactoryInterface
  • ObjectRepositoryFactoryInterface

let me know what do you think, thanks to the interfaces I created a stupid orm.

namespace ChangeSet;

//@todo make a cleaner solution?
abstract class Associator
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the correct approach. Instead of using inheritance, we can simply build an abstract factory that is able to use class metadata (and eventually a manager registry) to get the correct data. Please keep all this working with stdClass for now, since that's a point of extension (not meant for this repo)

@Ocramius
Copy link
Owner

@liuggio I generally like the changes, but don't try to specialize the factories or make them smarter for now. The demos are supposed to work with only stdClass, and the factories should be completely replaced by a consuming library configuring a micro-DIC that provides all these components.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 9d6c578 on liuggio:master into 782620b on Ocramius:master.

@liuggio
Copy link
Contributor Author

liuggio commented Jun 25, 2013

totally agree with you I see now the future :)

@Ocramius
Copy link
Owner

@liuggio can you remove the associator stuff then? :)

@liuggio
Copy link
Contributor Author

liuggio commented Jun 26, 2013

Yep!

On Wed, Jun 26, 2013 at 5:48 PM, Marco Pivetta notifications@github.comwrote:

@liuggio https://github.com/liuggio can you remove the associator stuff
then? :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-20057888
.

liuggio_______________

/|_ We reject kings, presidents and voting.
/o ) / We believe in rough consensus
)
v _/\ and running code (I.E.T.F. credo)


@liuggio
Copy link
Contributor Author

liuggio commented Jun 26, 2013

Ok
think about it as super WIP, I made this in half an hour and I'm in hurry (sorry) don't hate me, I added Pimple, and few containers, I'd like to change the names, and I removed the ObjectLoaderFactory only because I didn't not understand his objective.

@liuggio
Copy link
Contributor Author

liuggio commented Jun 26, 2013

If you think that Pimple is not necessesary I'll remove it leaving only the repo and the 2 interfaces..

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 1e7040b on liuggio:master into 782620b on Ocramius:master.

public function __construct($object, $compute = false)
{
$this->object = $object;

if ($compute) {
$this->snapshot = (array) $this->object;
$this->snapshot = (array)$this->object;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh, don't change those please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 05da51a on liuggio:master into db04fc8 on Ocramius:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 840bf70 on liuggio:master into db04fc8 on Ocramius:master.

@liuggio
Copy link
Contributor Author

liuggio commented Jun 27, 2013

Little recap:

  • The Factory are called factory but they have an interface because they'll be injected and their logic 'd change.
  • Object Loader Factory has been eliminated.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling feb324a on liuggio:master into db04fc8 on Ocramius:master.

@Ocramius
Copy link
Owner

Ocramius commented Jul 6, 2013

Merging - hopefully going on with this tomorrow (I will try to integrate it with the existing ORM ;) )

Ocramius added a commit that referenced this pull request Jul 6, 2013
[WIP] Pimple, Interfaces
@Ocramius Ocramius merged commit f1b1e82 into Ocramius:master Jul 6, 2013
@Ocramius
Copy link
Owner

Ocramius commented Jul 6, 2013

And obviously: thanks!

@Ocramius Ocramius added the proto label Dec 13, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants