search

Joey9801 / igc-rs

MIT License
6 stars 4 forks source link

Extract `ExtensionRange` struct #16

Closed Turbo87 closed 5 years ago

Turbo87 commented 5 years ago

This PR extracts an ExtensionRange struct from the existing Extension struct to simplify the get_extension() method signature. It also extracts a constructor for the Extension struct and moves the IRecord and JRecord structs to dedicated files to make them a little more discoverable.

Joey9801 commented 5 years ago

I'm not sure I agree with splitting out the range part of the Extension struct. If there was a case for an ExtensionRange to exist without a parent Extension (or an Extension to exist without an ExtensionRange for that matter) I might feel a little differently. At the moment they're so tightly coupled it doesn't make sense for them to be independent structures. Plus Extension only has three fields, it's not exactly unmanageably large ;)

Turbo87 commented 5 years ago

Plus Extension only has three fields, it's not exactly unmanageably large

it's not size that's the problem, it's more the unnecessary lifetime management that seems a little annoying to me 🤔

Turbo87 commented 5 years ago

@Joey9801 any thoughts on the other commits in this PR?

Joey9801 commented 5 years ago

Constructors are good, moving I/J records into their own modules is good. I've just merged both of those things - try to avoid conflating too many different things in a single PR.