2

I have successfully implemented FizzBuzz in angular, and was wondering if I am doing everything according to Angular Best Practices. My questions: 1) Is there any way to set $scope.display in the factory directly instead of returning something? so instead of "return FIZZ" could I do $scope.display = "FIZZ" there? 2) do i really need separate $scope.counter and $scope.display variables?

Code:

angular.module('fizzbuzz', [])


.factory("Counter", function() {
  var increment = function(number) {
    if (number % 3 === 0 && number % 5 === 0) {
      //any way to set $scope.display directly here?
      return "FIZZBUZZ"
    }
    if (number % 3 === 0) {
      return "FIZZ"
    }
    if (number % 5 === 0) {
      return "BUZZ"
    }
    return number;
  }
  return {
    increment: increment
  }
})

.controller("FizzBuzz", function($scope, Counter) {
  // is there any way to do this without a separate counter variable?
  $scope.display = 0;
  $scope.counter = 0;
  $scope.increment = function() {
    //increment the counter before going into the function, reacting to ng-click
    $scope.counter++;
    //call the factories function to actually display
    $scope.display = Counter.increment($scope.counter);
  }
})

//HTML:

<doctype! html>
<html>
<head>
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.19/angular.min.js"></script>
<script src="fizzbuzz.js"></script>
</head>
<body>
    <div ng-app="fizzbuzz" ng-controller="FizzBuzz">
        <h1 ng-click="increment()"> {{ display }} </h1>
    </div>
</body>
</html>

Update:

Took Shaun's suggestion to try this but it did not display anything:

angular.module('fizzbuzz', [])


.factory("Counter", function() {
  var service = {};
  service.number = 0;
  service.display = "";
  service.increment = function() {
    service.number++;
    if (service.number % 3 === 0 && service.number % 5 === 0) {
      //any way to set $scope.display directly here?
      service.display = "FIZZBUZZ"
    }
    if (service.number % 3 === 0) {
      service.display = "FIZZ"
    }
    if (service.number % 5 === 0) {
      service.display = "BUZZ"
    } else {
      service.display = service.number;
    }
  }

  return service;
})

.controller("FizzBuzz", function($scope, Counter) {
  // can reference method and data from the service
  $scope.increment = Counter.increment;
  $scope.display = Counter.display;
})
cdlane
  • 40,441
  • 5
  • 32
  • 81
devdropper87
  • 4,025
  • 11
  • 44
  • 70
  • 1
    seems like this is better suited for http://codereview.stackexchange.com/ – Neil S Sep 24 '15 at 22:29
  • @NeilS good point will submit there next time! – devdropper87 Sep 24 '15 at 22:48
  • updated my post with a runnable snippet problem is when you copy the values there you aren't using a reference through the original object so updates to the original objects properties don't update those copies – shaunhusain Sep 25 '15 at 02:26

2 Answers2

1

angular.module('fizzbuzz', [])
    
    
    .factory("Counter", function() {
      var increment = function() {
        service.number++;
        if (service.number % 15 === 0) {
          //any way to set $scope.display directly here?
          service.display = "FIZZBUZZ"
        }
        else if (service.number % 3 === 0) {
          service.display =  "FIZZ"
        }
        else if (service.number % 5 === 0) {
          service.display =  "BUZZ"
        }else{
          service.display = service.number
        }
      }
      var service = {
        increment: increment,
        number:0,
        display: 'Click to start'
      }
      return service;
    })
    
    .controller("FizzBuzz", function($scope, Counter) {
      // can reference method and data from the service
      $scope.Counter = Counter;
    })
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>
Testing
<div ng-app="fizzbuzz" ng-controller="FizzBuzz">
  <h1 ng-click="Counter.increment()"> {{ Counter.display }} {{ Counter.number }} </h1>
</div>

Modified to include an HTML example of usage. Also modified the FIZZ BUZZ logic based on the explanation here: http://c2.com/cgi/wiki?FizzBuzzTest

shaunhusain
  • 19,630
  • 4
  • 38
  • 51
  • I tried your approach but it didn't work--- given the html, nothing is showing up on the screen at all. Also, thanks for the clarification on $scope no being in the factory. – devdropper87 Sep 25 '15 at 01:44
  • Editing my code to show what I tried from your suggestion. – devdropper87 Sep 25 '15 at 01:47
  • Sorry for the initially short post was trying to do it on my phone so couldn't really test just had to write it mostly blind. updated with a runnable snippet – shaunhusain Sep 25 '15 at 02:28
  • thanks and no problem. the thing is though I have to display only the number or only the text (not both at the same time). thanks clarifying on the service! – devdropper87 Sep 25 '15 at 03:21
0

Your questions: 1) Is there any way to set $scope.display in the factory directly instead of returning something? so instead of "return FIZZ" could I do $scope.display = "FIZZ" there?

NO, you did well !

2) do i really need separate $scope.counter and $scope.display variables?

In your controller, you don't need to put your counter variable inside your scope. It's just an increment variable.

Your increment() function can return your display number as well:

JS

.controller("FizzBuzz", function($scope, Counter) {

  // is there any way to do this without a separate counter variable?
  var counter = 0;

  $scope.increment = function() {
    counter++;
    return ( Counter.increment(counter) );
  }
});

HTML

<html>
<head>
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.19/angular.min.js"></script>
<script src="fizzbuzz.js"></script>
</head>
<body>
    <div ng-app="fizzbuzz" ng-controller="FizzBuzz">
        <h1 ng-click="increment()" ng-bind="increment();"> </h1>
    </div>
</body>
</html>
Nicolas2bert
  • 1,142
  • 7
  • 10