elixirlabsinc / endslaverynow

Creating the End Slavery Now project
2 stars 0 forks source link

Click views #31

Closed pedanticantic closed 5 years ago

pedanticantic commented 5 years ago

See also the issue that this PR is for.

I have refactored the code to save a single record instead of the entire database when anything is changed.

This involved rationalising the various ways the application saves data, so now it uses $add(), $save() and $remove(). As a result, the record collection keys are now strings instead of integers - I believe is the correct way of doing it.

It no longer uses the syncObject (well almost never). An added bonus is that there are essentially no errors in the console now (there are one or two, but they're not related to saving/loading data).

There are a lot of controllers where 1 line ("$scope") has been removed. app/scripts/services/DataRepositoryFactory.js has had a lot of redundant code removed. The main changes are in app/scripts/services/DataRepository.js.

I made a couple of small improvements/changes whilst doing this:

pedanticantic commented 5 years ago

@toriejw , in reply to your question about the console.logs, I have a vague memory that someone (possibly Aashni?) said that they were essentially placeholders because the error handling hadn't really been implemented. Obviously the end users aren't going to see them (unless they have the developer console open, which is fairly unlikely), so at the moment only developers will see them. Perhaps we need to suggest error handling as another task to do within the project?

pedanticantic commented 5 years ago

Oh, @toriejw , I was also going to say that I am worried/annoyed that there is still quite a lot of duplicated (well very similar) code after my big refactor. You'll learn that one of my pet hates is duplicated code!! For example, there are 3 methods, determineIndexFrom<record-type>Model() that have identical logic in them, but just reference a different array/model. In theory they could be combined into a single method, but the code would be horribly dynamic, hard to understand and prone to subtle bugs. Do you have any ideas how we could reduce the duplication? I think it's okay for now, but might be a problem in the long term.

toriejw commented 5 years ago

Makes senes on the duplication. I don't have any other ideas currently, but I'll keep it in the back of my head and see if anything comes up :) .