-
Notifications
You must be signed in to change notification settings - Fork 0
Guntt#3 #17
base: master
Are you sure you want to change the base?
Guntt#3 #17
Conversation
gunttTimeAndDateCtrlModule.controller('gunttDateAndTimeCtrl', | ||
function ($scope, $interval, $log, toastr, $window, $document) { | ||
|
||
//$scope.test = toastr.success('Time And Date Ctrl successfully loaded'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest not to put comments like this to version control, makes harder to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in release
|
||
//GET USER DATA MODEL | ||
$scope.gunttUserData = ResourceService.getCurrentUser().then(function (data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we use Resolver
service instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
Neither app ctrl does use it, but calls promises by chaining (then()...)
Resolver is just a wrapper for promises/deffered objs
So I use direct call for service, which returns promise, .then() I get resolved data and use it further..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, because Resolver
is a wrapper to simplify resolving various promises, as far as I can see. Basically my understanding is that Resolver
intended to be used for async preps of any data for controllers instead of it's constructors.
I am not familiar with this side of Angular and not sure which approach is better, but for sure I can say and will insist that we keep things unified as much as possible, so either we use Resolver
everywhere possible or not using it at all and throw it away from the current code base. Something tells me this could be related to no-scope origins of our prototype so I am not entirely against of changing approach, but we must know for sure. Could you please perform some research on that one and share your findings, i.e. what is Resolver
pros and cons and what would be a valid use cases for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as I found, $q functions is the collection of promises either in array, or in a deffered object, which you can pass to Resolver wrapper, and it will wait till all promises are resolved (because return $q.all(promises), then proceed next.
But another way(for me more simple) is just to use a chain of promises by "then().then()...."
// form Angular DOC//
Because calling the then method of a promise returns a new derived promise, it is easily possible to create a chain of promises:
promiseB = promiseA.then(function(result) { return result + 1; }); // promiseB will be resolved immediately after promiseA is resolved and its value // will be the result of promiseA incremented by 1
It is possible to create chains of any length and since a promise can be resolved with another promise (which will defer its resolution further), it is possible to pause/defer resolution of the promises at any point in the chain. This makes it possible to implement powerful APIs like $http's response interceptors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this moment I cannot see the valuable reason to use the Resolver Service, as we can simply use .then()
instead..
|
||
/*USER CONSTRUCTOR **START** */ | ||
function GunttUserService(ResourceService) { | ||
this._ResourceService = ResourceService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to do it this.resourceService
, this is a multi-language pattern when class names are capitalized and variables are not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'_' between developers means 'protected' property, like "do not change"
-get first user data
-user icon change