Hi Clinton,
Here's an example for the struct of function
approach: https://play.golang.org/p/F_f0hLNsmSW
These are not key issues, I'm just sharing the path I've been on.
Thanks for sharing Graven here. I'll read those references.
Matt
On Wednesday, January 24, 2018 at 3:57:27 PM UTC-6, matthe...@gmail.com
wrote:
>
> You are using interfaces to define a type. The overhead of interface isn’t
> necessary to achieve the same goal. I see that github.com/gorilla also
> uses this pattern in places. Supporting another tool or providing a mock
> for testing is just making a new value of the type.
>
> I do use sub-packages and encapsulation to simplify working with certain
> concepts, but here my opinion is the use of encapsulation is not adding
> much value. My experience has been that Go utility functions are more
> easily copy-pasted into each project rather than shared, although for an
> ecosystem of applications I can see that shared packages would form. But
> for this project the effort seems too early. My opinion is make the package
> when you need it.
>
> Matt
>
> On Wednesday, January 24, 2018 at 10:06:17 AM UTC-6, Clinton Begin wrote:
>>
>>
>> Hi Matt, thanks for the review.
>>
>> Here is some feedback and explanation.
>>
>> >> Does BuildTool need to be an interface? Having methods on a pointer to
>> nothing (type GoBuildTool struct{}) seems unnecessary to me.
>>
>> Interfaces are expressions of functional contract, and have little
>> dependency or basis for requiring state (fields on a struct). BuildTool is
>> an interface for two reasons. The first is that there is potential to
>> support other build tools other than the go executable. I briefly
>> considered supporting gb for example. But also, who knows. Someday maybe
>> there will be a gnu go compiler or some such thing. The second reason is
>> unit testing. While I don't mock that interface in any of the current unit
>> tests, this allows me to if I need to at some point. In some languages
>> performing an extract interface refactoring after the fact is easy. Golang
>> is not one of those languages. So erring slightly on the side of interfaces
>> is worth it, especially where there are clear cases for alternative
>> implementations or test isolation from external dependencies (both
>> applicable in this case).
>>
>> >> Also, why not have this at the level of package main?
>> >> Same with graven/commands, graven/config, graven/domain, graven/util,
>> since these are specific to
>> >> graven they could also be part of package main. Config is just
>> defining one type.
>>
>> As a practice I try to keep as much out of main as possible. Not only
>> does it keep code more organized as the project grows, as in any other
>> language, but in Go it also means that I can access this code as a library
>> from other potential tools or test harnesses. I don't let the number of
>> files dictate what should be a package or not. It's functional context that
>> contributes to that decision.
>>
>> >> interface makes sense when just grouping behavior with no data
>> structure
>>
>> Again, interfaces have nothing to do with state or data. An interface is
>> a functional contract. It says: Any such thing that deems itself as
>> implementing an interface (implicit in Go), must implement these functions.
>> There is no reliance or direct relationship to state, either by design,
>> contract or even pattern. In fact, more often than not, interfaces describe
>> the core logic of a system or interfaces to external systems, and having
>> mutable state in such implementations leads to a source of bugs in the form
>> of shared state, race conditions and locking. For this reason, many
>> interfaces choose to accept their state as parameters to their functions,
>> rather than store them as part of the implementation.
>>
>> I recommend this blog post to learn more:
>> http://jordanorelli.com/post/32665860244/how-to-use-interfaces-in-go
>>
>> >> Well-named package main files and types provide just as good logical
>> separation.
>>
>> I disagree here. Golang has very weak encapsulation. There are very few
>> options for protecting access to fields and functions. Go only has package
>> level access rules, and therefore packages do a lot more for accessibility
>> than is necessary to consider in other languages. In C# or Java, sure, you
>> could put every file in one package or directory and protect access to
>> fields and functions. But you still wouldn't do that for the sake of code
>> organization and even in Java, scoping as well (protected is also package
>> scoped). So I disagree with your suggestion here in Go and any other
>> language really.
>>
>> >> graven/resources/commmon is misspelled. I’m not sure why you have
>> these simple text files when a const in code
>>
>> Well that is embarrassing. :-) Those are old test fixtures that I don't
>> think are used anymore. I've since moved test fixtures to the test_fixtures
>> directory, but those seem to have been left behind. Thanks for catching
>> that.
>>
>> Cheers,
>> Clinton
>>
>> On Sunday, January 21, 2018 at 9:36:57 AM UTC-7, matthe...@gmail.com
>> wrote:
>>>
>>> Hi Clinton, here’s a code review.
>>>
>>> Does BuildTool need to be an interface? Having methods on a pointer to
>>> nothing (type GoBuildTool struct{}) seems unnecessary to me. Also, why not
>>> have this at the level of package main?
>>>
>>> Same with graven/commands, graven/config, graven/domain, graven/util,
>>> since these are specific to graven they could also be part of package main.
>>> Config is just defining one type.
>>>
>>> Since graven/domain.Project, Artifact, Repository, Version only has read
>>> operations defined why not have the struct as method receiver instead of
>>> pointer to the struct? I’d only move to the pointer for performance if
>>> proven by a testing benchmark, pprof profile, and need, otherwise no
>>> pointer is more understandable/readable.
>>>
>>> Again in graven/repotool, graven/vcstool, I’m not sure having an
>>> interface makes sense when just grouping behavior with no data structure.
>>> Defining a struct type with function fields with a var for docker, github,
>>> and maven may be better. These packages are also application specific;
>>> packages should be portable and not just used to group application logic.
>>> Well-named package main files and types provide just as good logical
>>> separation.
>>>
>>> graven/resources/commmon is misspelled. I’m not sure why you have these
>>> simple text files when a const in code could maybe provide the same
>>> functionality.
>>>
>>> Matt
>>>
>>> On Saturday, January 20, 2018 at 10:54:29 PM UTC-6, Clinton Begin wrote:
>>>>
>>>> Hi all,
>>>>
>>>> I'm looking for feedback on a build tool I created to fill some gaps I
>>>> saw in the Golang project build space. While I've used this tool for 9
>>>> months or so, you can very much consider it a prototype of sorts, or a
>>>> concept, and PRs are definitely welcome. This is hopefully the beginning
>>>> of
>>>> a conversation, not an assertion that this is a finished product.
>>>>
>>>> https://github.com/cbegin/graven
>>>>
>>>> There's an intro video and short design motivation doc. Remember that
>>>> motivation works both ways. It influences both things we want to emulate,
>>>> and things we want to avoid. So please don't be put off immediately by the
>>>> Maven references. While Maven as a tool can be a nightmare, as a concept,
>>>> it's purpose, goals and what it's proven in the Java space, are worth
>>>> learning from.
>>>>
>>>> Cheers,
>>>> Clinton
>>>>
>>>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
