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

pila: allow publicly to use custom Stack implementation #47

Merged
merged 6 commits into from Feb 17, 2017

Conversation

fern4lvarez
Copy link
Owner

@fern4lvarez fern4lvarez commented Feb 17, 2017

This commit does not imply API breaking changes, so it can be added in the next
0.1.X release.

Now it is possible to create a pila.Stack using a custom
implementation that satisfies the stack.Stacker interface, e.g.

type TestStack struct{}

func (s *TestStack) Push(element interface{}) { return }
func (s *TestStack) Pop() (interface{}, bool) { return nil, false }
func (s *TestStack) Size() int                { return 0 }
func (s *TestStack) Peek() interface{}        { return nil }
func (s *TestStack) Flush()                   { return }

stack := pila.NewStackWithBase("stack", time.Now(), &TestStack{})

It is also possible to create a Stack with a custom base implementation from a Database instance:

id := db.CreateStackWithBase("stack", time.Now(), &TestStack{})

This commit does not imply API breaking changes changes.

Now it is possible to create a `pila.Stack` using a custom
implementation that satisfies the `stack.Stacker` interface, e.g.

```go
type TestStack struct{}

func (s *TestStack) Push(element interface{}) { return }
func (s *TestStack) Pop() (interface{}, bool) { return nil, false }
func (s *TestStack) Size() int                { return 0 }
func (s *TestStack) Peek() interface{}        { return nil }
func (s *TestStack) Flush()                   { return }

stack := pila.NewStackWithCustomImplementation("stack", time.Now(), &TestStack{})
```
@codecov-io
Copy link

codecov-io commented Feb 17, 2017

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #47   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          18     18           
  Lines         660    664    +4     
=====================================
+ Hits          660    664    +4
Impacted Files Coverage Δ
pila/stack.go 100% <100%> (ø)
pila/database.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5447be8...8f9b520. Read the comment docs.

@fern4lvarez
Copy link
Owner Author

Thanks @javierprovecho for the feedback on this subject. Feel free to review the changes.

pila/stack.go Outdated

// NewStackWithCustomImplementation creates a new Stack given a name, a creation date,
// and a stack.Stacker implementation without an association to any Database.
func NewStackWithCustomImplementation(name string, t time.Time, stack stack.Stacker) *Stack {

Choose a reason for hiding this comment

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

Maybe don't shadow stack package with stack argument, and rename it like impl, s or base. You get the idea...

Copy link
Owner Author

Choose a reason for hiding this comment

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

I exactly had this thought when I named the parameter as stack. So if a second mind think likewise, change the name it is.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@fern4lvarez
Copy link
Owner Author

fern4lvarez commented Feb 17, 2017

@javierprovecho thanks! we'll discuss about making public the Stack.base field for a non-PATCH release 😉

@fern4lvarez fern4lvarez merged commit 3d19e54 into master Feb 17, 2017
@fern4lvarez fern4lvarez deleted the stack-custom branch February 17, 2017 20:36
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.

None yet

3 participants