-
Notifications
You must be signed in to change notification settings - Fork 10
What are the benefits of a types package? #61
Comments
I'm not sure I understand what you mean by |
@liamsi Exactly that, sorry for failing to provide proper reference, amended |
Eager to hear your strong opinions/ alternative designs to the types package. I think in the context of the SDK - types is nice explicitly for when trying to reference an interface. If we limit the implementations of the types to outside of the types package I'd imagine it's a maintainable and safe development pattern. I imagine that clumping to much into a single package could raise issues down the road (thoughts here?) - but also I to much package (and file) separation can be a real hassle to work with and create obfuscated code bases which are difficult to navigate and understand - which is a real issue. |
@melekes The article doesn't mention a types package, rather talkes about a root package and sub-packages. |
Yeah typically the design pattern I've seen is where folks use the root directory kind of like a types package, What's your opinion on this @xla vs straight up |
IMHO there is only one valid application of the a package called Resorting to something like types.FooBar
types.FooBaz
///
foo.Bar
foo.Baz My point is that when using something as generic as
In reality types seldomly stand in isolation and are almost always bound with functionality, which end up in the same package, which leads to that code needed to be tested. If we assume that the Dependency inversion principle has any value then ideally interfaces should be declared where they are needed, while there are strong examples of widely used interfaces, e.g. |
I don't feel so strong about the As far as I understand, the structs there are the ones which will be send around via JSON / amino? Is that correct? If so, why no call it |
@liamsi lol we need need to name something "aminos" way to amazing |
Disclaimer: Cosmos-SDK Centric @xla great point about the additional namespace overhead - totally with you on that one. Not sure what this looks like in tendermint but in the SDK there actually ARE "types that stand in isolation of functionality". In this situation the types are defined in the sdk and utilized by independent modules (with the intention of being able to mix/match swap out modules) - So in this particular instance - I actually feel that it makes sense to have these high level type definitions (intended to be fulfilled by arbitrary modules) separated from their functionality. For everything else that is not explicitly what I just mentioned I whole-heartedly agree that it doesn't belong in the What are your thoughts on that? |
The "types" folder is there in place of putting all the files in the root directory. Keeping project-wide common types esp for external callers in the root directory is the standard convention for Golang. We want to keep the root directory clean, Makefile and Godep.toml etc., so moving every go file from the root directory to a special directory is IMO an improvement over existing Golang conventions. "util" is a catchall for internal implementations and it does get unwieldy. But the SDK itself IMO should have a catch-all package called "sdk" where a lot of common interfaces/structs live. Handler, AnteHandler, FeeHandler, Context, Address, Account, AccountMapper, Coin/s, Error, StdSignature, Tag/s, Tx, Msg, StdSignDoc, and all the store interfaces, I believe they should be referred to as "sdk.*" so we can standardize around these common interfaces and structures. They're all common elements that one needs to know about to fully grasp the SDK, as the SDK is built today. It's true that others can build alternative systems using the components we've already built... like a ParallelBaseApp (Sunny's suggestion). But that shouldn't prevent us the SDK having specific opinions today that get someone started on building a kickass app. Most people don't need parallel tx processing. The types defined in "types" and the opinions that it represents, should be sufficient for the majority of uses-cases. BaseApp should be the structure that users embed in their CustomApp. So BaseApp should embed IAVLApp, that seems fine. IAVLApp can live in cosmos-sdk/iavlapp or something. So we can have /baseapp and /iavlapp. |
cool thx |
@jaekwon Could you reference where this convention orignates from? Personally I've never seen this to be the case and even light inspection of popular Go projects shows a total different approach. If anything people are advocating for package division by domain, What are the main concepts, where do they interface, etc. And then have their types close to them and not remote in another place. To give some context the CodeReviewComments explicitely state to avoid meaningless package names. Some more thoughts on packages:
In the end I don't feel strongly about this issue because I want to see conventions, best-practices or the soup du jour being followed, but I strongly believe that this has real implications on the quality and complexity of the software we write. As we would benefit from the constant conversation how something fits into the domain and what it exposes to the world. |
Personally I feel very strongly about
types
and would go as far as call it an anti-pattern. Happy to go into detail why, but before curious to hear good arguments for the advocate pratice.Reference in the tendermint codebase: https://github.com/tendermint/tendermint/tree/master/types
The text was updated successfully, but these errors were encountered: