Skip to content

Extract Socket constants in separate file.#6717

Closed
straight-shoota wants to merge 1 commit intocrystal-lang:masterfrom
straight-shoota:jm/refactor/socket-extract-common
Closed

Extract Socket constants in separate file.#6717
straight-shoota wants to merge 1 commit intocrystal-lang:masterfrom
straight-shoota:jm/refactor/socket-extract-common

Conversation

@straight-shoota
Copy link
Copy Markdown
Member

@straight-shoota straight-shoota commented Sep 13, 2018

Socket::Address and Socket::Addrinfo conceptually don't dependent on
Socket. The shared constants and lib includes are extracted to common.cr to separate concerns more clearly.

@straight-shoota straight-shoota force-pushed the jm/refactor/socket-extract-common branch 2 times, most recently from 8de9343 to 8755a3a Compare September 13, 2018 20:04
`Socket::Address` and `Socket::Addrinfo` conceptually don't dependent on
`Socket`. The shared constants and lib includes are extracted to common.cr to separate
these concerns more cleary.
@straight-shoota straight-shoota force-pushed the jm/refactor/socket-extract-common branch from 8755a3a to 56e3b97 Compare September 13, 2018 20:07
@ysbaddaden
Copy link
Copy Markdown
Collaborator

I dont understand. If they're common constants, that they'll be included anyway, and they're still named Socket::X then I expect them to be defined in socket.cr not buried in some other file.

Can we avoid stylistic/opinion changes, and concentrate on fixing actual issues in Socket?

@straight-shoota
Copy link
Copy Markdown
Member Author

This is not about style, it is about separation of concerns. I'd like socket.cr to focus on the Socket type, not some other stuff that is in its namespace but not solely required by Socket.
The idea is, if you only need Socket::Address, you can just include socket/address instead of the entire package.
While this selective requiring doesn't have any relevant performance impact as it might have in other languages, the expression of explicit dependencies is still valuable.

This PR is in preparation for refactoring the entire socket API. In the process I found it was much easier to separate individual pieces of code into separate files instead of having the main type + some common stuff in the main file.

This PR is not an integral part of the refactoring, that's why I published it separately. But it makes refactoring easier.

@jhass
Copy link
Copy Markdown
Member

jhass commented Apr 29, 2020

So, where do we want to go with this? 18 months later it didn't seem to have been a pain point for anybody given the activity here...

@straight-shoota
Copy link
Copy Markdown
Member Author

Closing stale PR.

@straight-shoota straight-shoota deleted the jm/refactor/socket-extract-common branch November 19, 2020 16:17
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.

3 participants