Task: Within a controller I have a default object which needs to be modified based on a number of conditions. The modification logic is repetitive hence I ported it to a service/ utility called colorService (can create a private method as well). The ask is to set only one color to true and others to false.
Case 1:
//CONTROLLER
$scope.colorObj = {
red:true,
green:true,
blue:true,
white:true,
yellow:true
};
$scope.changeColor = function(){
var color = 'r';
switch(color){
case 'r':colorService.setColor('red',$scope.colorObj);break;
case 'b':colorService.setColor('blue',$scope.colorObj);break;
case 'g':colorService.setColor('green',$scope.colorObj);break;
case 'w':colorService.setColor('white',$scope.colorObj);break;
case 'y':colorService.setColor('yellow',$scope.colorObj);break;
default: colorService.setColor('',$scope.colorObj);break;
}
}
//SERVICE
colorService.setColor = function(type, colorObj){
angular.forEach(colorObj,function(val,color){
if(color === type){
colorObj[color] = true;
}else{
colorObj[color] = false;
}
});
};
If I follow this approach, then the changes get reflected on the $scope.colorObj immediately as the object reference is shared. Hence a 'return' is not needed.
But this approach passes the $scope to the service as an argument which I know is not a best practice. Also, the service method doesn't have a return statement which goes against the grain of a service as well.
Case 2:
//CONTROLLER
$scope.colorObj = {
red:true,
green:true,
blue:true,
white:true,
yellow:true
};
$scope.changeColor = function(){
var color = 'r';
switch(color){
case 'r': $scope.colorObj = colorService.setColor('red');break;
case 'b': $scope.colorObj = colorService.setColor('blue');break;
case 'g': $scope.colorObj = colorService.setColor('green');break;
case 'w': $scope.colorObj = colorService.setColor('white');break;
case 'y': $scope.colorObj = colorService.setColor('yellow');break;
default: $scope.colorObj = colorService.setColor('');break;
}
}
//SERVICE
colorService.setColor = function(type){
var colorObj = {
red:true,
green:true,
blue:true,
white:true,
yellow:true
};
angular.forEach(colorObj,function(val,color){
if(color === type){
colorObj[color] = true;
}else{
colorObj[color] = false;
}
});
return colorObj;
};
In this case, I have isolated the $scope but had to add a local object colorObj in the service method which is returned and copied onto the $scope.colorObj.
I find this more testable but again I need to maintain multiple colorObj in controller and service.
Case 3:
//CONTROLLER
$scope.colorObj = {
red:true,
green:true,
blue:true,
white:true,
yellow:true
};
$scope.changeColor = function(){
var color = 'r';
switch(color){
case 'r': setColor('red');break;
case 'b': setColor('blue');break;
case 'g': setColor('green');break;
case 'w': setColor('white');break;
case 'y': setColor('yellow');break;
default: setColor('');break;
}
};
setColor = function(type){
angular.forEach(colorObj,function(val,color){
if(color === type){
$scope.colorObj[color] = true;
}else{
$scope.colorObj[color] = false;
}
});
};
Here, I can simply create a private method setColor within the controller to get things done. The downside is that setColor is not uniquely testable; yes, it can be tested by initiating a test for the parent method changeColor
Which option would be a better one?
