search

gurtejboparai / bookwyrm

A project for Software Engineering II WInter 2022
2 stars 3 forks source link

Create a single point of truth for the list of genres #209

Closed CameronJung closed 2 years ago

CameronJung commented 2 years ago

The list of genres is used in multiple files as read only data. It would be wise to put this data into a single point of truth.

LukeBMorrow commented 2 years ago

My suggestions are either add it to the Vue store such that they are global, or even better, add them to the DB with the other data.

CameronJung commented 2 years ago

My issue with vue store is that puts it in local storage doesn't it? I don't think we should put immutable static data in the local storage since it doesn't need it to persist when the user isn't on our site. Plus it's the memory for somebody else's computer, so I think it should be justified. The backend also isn't a great place to store this data. As far as I can tell the genre list isn't used anywhere in the backend, if that weren't the case it would absolutely be where it should go, but at this point the only justification I can think of is future proofing. There is also an issue with using a purely storage based solution in general because we have one place where it wants the genre list without the "overall" entry. What if we made a singletons folder in src and put it there? My current design for issue #205 is another front-end singleton so it's not like the folder would be for a single file.

LukeBMorrow commented 2 years ago

The store doesn't use localStorage if that's what you mean. Vue runs on the client side, so long as you want it stored on the front end, in any capacity, it will be taking up the client's RAM.

If we want a thin client, we should be storing it in the DB (not to harp on about best practices, but this is preferable by the SOLID principle). If we dont mind the client side storage, it should be used as a global static constant since it's just one variable. The store works for storing the global constants, but there are other ways too, I just don't know them. If it were in its own file, it would likely be a top level contants.js file.

Side Note: I'm also not sure how you are planning to do #205, but I'd imagine that would be a service with a method for sanitization (even if it's only a single line).

CameronJung commented 2 years ago

Alright, if the store is only using storage/memory when the site is actually being used, than that will be a great place for the genre list and I'll change my implementation to use that.

However, for the sake of my own learning I don't mind this discussion continuing in some manner because I think there's something to be gained from understanding how other people view, interpret and apply design principles. So I will try to explain my own reasoning. The SOLID principles pertain to object oriented programming, so they're relevant but not the only thing to consider. My perspective is coming from the distributed computing principle that; "data should be stored where it used/changed", if there's a name for this principle I don't remember it. In the front end the genre's being in a list is used to render elements, and the fact that they are in a list is irrelevant everywhere else in the application, hence why the list structure is broken up into a table which is more useful to the backend.

I also see why having the list in the backend would be better for data stability, but I think these concerns are beyond the scope of this project.

LukeBMorrow commented 2 years ago

I don't recall being taught that as an architecture principle in distributed computing, it sounds like they might be referring to a p2p system(which ours is not) given such a principle would induce a CAP crisis in a central coordinator system if the data were to change on a client. It was taught as a principle in object orientation I believe, and meant class information should be grouped by cohesion. In that case, I am of the opinion that the list of available genres is app data and thus has the best cohesion in the database alongside the other app data.

Storing the data locally in the client would likely be the easiest solution to implement, but is an antipattern for an extendable system. But like you said, it's not really a concern for this project seeing as we'll not be extending it.

CameronJung commented 2 years ago

It's possible that principle was meant for P2P applications. I also can't find it in my notes so I'd be willing to give adding the genre list to the backend a try, its important to do things the right way I just don't think having it on the client is necessarily wrong in this case where our design involves this list not changing.

However, while I was researching this a thought came to me, "how does the genre list get to the client in the first place?" What were supposed to be making is a web app and from finally googling "web app vs website" I came across this bit of information. My understanding of web apps may be wrong and I'll take your word for it if it is. However, from what I read the client-side programming is sent to the client when they visit the site. So since the list of genres is hard coded then the server updates the client whenever they visit thus transferring any changes that were made on the server. Hence, there shouldn't be an issue with that, because the single point of truth is still coming from the server. If this is accurate than putting the genre list in the backend would be redundant.

LukeBMorrow commented 2 years ago

I'd say do whichever you can get done faster, we have a lot of changes to make and very little time.

There are some systems that update on every page load, or sometimes mid-page use, but that's not every app. Many require the user logs out and back in to get the changes, which would be problematic if the user has a "Remember me" login function enabled.