Follow users dev #53
Follow users dev #53
Conversation
|
|
| @@ -22,7 +22,8 @@ export class ViewProfilesComponent implements OnInit { | |||
| constructor( | |||
| public fAuth: AngularFireAuth, | |||
There was a problem hiding this comment.
So in other files I've noticed that you have private fAuth vs public fAuth here. I'm wondering if they should all be one or the other?
There was a problem hiding this comment.
Oops yes you are right! I totally forgot that I noticed that and should change it. Thank you for pointing it out! I believe that Private vs Public shouldn't actually matter all that much in Typescript, but it is better to be consistent.
| return false; | ||
| } | ||
|
|
||
| follow() { |
There was a problem hiding this comment.
These follow() and unfollow() functions seem like good candidates for unit testing coverage
There was a problem hiding this comment.
Yes, I will be sure to make unit testing for them. Thank you!
| @@ -18,7 +18,7 @@ export class LoginComponent implements OnInit { | |||
| //and there is no user in the database linked to the account | |||
| //it will delete the user (in case user refreshes page before | |||
| //setting a username but after signing in with Google | |||
| constructor(public fAuth: AngularFireAuth, public router: Router) {} | |||
| constructor(private fAuth: AngularFireAuth, private router: Router) {} | |||
There was a problem hiding this comment.
It's really nice to limit visibility as much as possible up front! Thanks for switching a bunch of these away from public
| } | ||
| } | ||
|
|
||
| unfollow() { |
There was a problem hiding this comment.
Would be a bit cleaner to have one setFollowing(bool) method that had all the storage logic in it, and turn follow() and unfollow() into one-line methods that call setFollowing(true/false).
There was a problem hiding this comment.
Thanks for the idea, it definitely made the readability better and condensed the code.
This PR adds:
Something we might want is a map instead of an array. It will improve time complexity for following and unfollowing. However, if a User deletes their account, to remove this account from the following list would be harder. With an array you can use the .where("following", "array-containes", "uid") and get all of the documents very easily? This also is a less common practice though.