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

refactor rawClass to be more verbose #40

Open
reeceyang opened this issue Mar 21, 2023 · 6 comments
Open

refactor rawClass to be more verbose #40

reeceyang opened this issue Mar 21, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@reeceyang
Copy link
Contributor

previous discussion in #36 and #37

right now most of the rawClass property names are 1-2 letters. this helps reduce the size of latest.json etc. but it also hard to understand.

since the files are gzipped by the server it might not make a huge difference to rename the properties to be more verbose, but we'd have to try it out and see.

a different solution is to convert the json into some relational notation (i.e. each row is a field and each column is a rawClass field) before serving it, then parsing it back to json on the client side. this would store the data much more efficiently since we don't have to repeat the field names for every class. I kind of like this solution more; what do y'all think

@cjquines
Copy link
Member

yeah, the thing is that we're really serving a database to the client and we need to load it. our schema is designed in such a way that it could be a relational database.

i'm not super concerned about efficiency; i imagine the common use case is using it from mit, which has decent internet anyway. i'm more concerned about future developers understanding how the rawClass thing works, and i agree that single letter things make it hard to understand.

i think we should stick to something json based because that's what the courseroad api uses or smth. the ideal is that we eventually don't need to convert from the courseroad api at all

@reeceyang
Copy link
Contributor Author

that sounds good! sticking with json makes sense.

if the goal is to eventually not need to convert from the courseroad api, we could rename the fields according to the corresponding fireroad fields: https://fireroad.mit.edu/reference/catalog

fireroad uses snake_case though. since rawClass is just supposed to represent the raw data I think it's fine to keep the snake_case to be consistent (instead of converting to camelCase)?

I'm also down to work on the renaming once we decide on the new naming scheme!

@cjquines
Copy link
Member

@psvenk who's working on a new api

@psvenk
Copy link
Member

psvenk commented Apr 2, 2023

Right now I'm working on making a new backend that is compatible with the existing FireRoad API; the new backend can eventually also support a v2 API if necessary.

As for the frontend side (i.e., as far as Hydrant itself is concerned), renaming the property names to be more descriptive by matching the FireRoad API sounds like a reasonable course of action. The current property names are just a vestige of Firehose, which used its own scraper rather than interfacing with the FireRoad server, but there's no need to maintain backwards compatibility with that.

@reeceyang
Copy link
Contributor Author

sounds good, i'll work on a pr for the refactoring

@cjquines cjquines added the enhancement New feature or request label Sep 12, 2023
@reeceyang
Copy link
Contributor Author

so i never got around to doing this last semester - i haven't forgotten about it though and will probably have some time to do it in a few weeks or so, but if anyone else wants to pick it up in the meantime feel free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants