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

public #8

Open
wants to merge 2 commits into
base: kylef/nep-2
Choose a base branch
from
Open

Conversation

loganwright
Copy link

I think the BytesPayload object is useful as a public asset for quickly declaring PayloadConvertible types.

@kylef
Copy link
Member

kylef commented Feb 22, 2016

I can't think of many uses of BytesPayload that warrant this being public. Could you please elaborate with some example use-cases?

@loganwright
Copy link
Author

@kylef I'm less concerned with accessing the property, just didn't see a reason for it to be internal.

Mainly wanted to expose the initializer so quick byte payloads can be created w/ arbitrary data.

@kylef
Copy link
Member

kylef commented Mar 5, 2016

just didn't see a reason for it to be internal

Everything should be private API unless there is a good reason for it to be public. Once public we have to maintain backwards compatibility and it's harder to change or remove it in the future.

I think we can provide higher level API to create bodies from strings and files so I can't see any uses for this particular class to be public.

If there is a legitimate use-case for this to be public, then we can explore doing so at any time.

@loganwright
Copy link
Author

@kylef I think there's a bit of confusion. In the sentence quoted, I was referring specifically to the bytes property which I could go either way on for access control. Having it internal or private does limit api necessary for support. and seems like a potential benefit.

For the object, BytesPayload, having it public is an easy way to create generators from byte arrays in a way that conforms to PayloadType. I think the object covers the majority of uses w/o requiring users to implement their own version of it. In my implementation, I found having the object public to be a useful feature.

The other alternative would be to not accept a PayloadType and accept something like SequenceType where Generator.Element == UInt8 so that any arbitrary type could be passed in and the generator for that sequence would be created internally.

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