2
\$\begingroup\$

I created a simple Angular JS application for calculation gallons of paint needed to paint the ceiling of a room.

I assume one gallon covers 350 square feet and use it as a constant. Also, I round up a counting of the gallons to the next whole number. Now my code seems to me too simple).

I will be grateful for advice as it is possible to complicate this task with the use of AngularJS or next version of Angular.

angular.module('my-App', [])
	.controller('paintCalculatorController', function($scope){
		$scope.length = 0;
		$scope.width = 0;
		const squareFeetForOneGallon = 350;
		$scope.square = function(){
			return $scope.length * $scope.width;
		}
		$scope.countGallon = function(){
			return Math.ceil($scope.length * $scope.width / squareFeetForOneGallon);
		}
	});
form{
	background-color: #f1f1f1;
	border-radius: 5px;
	}
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.6/css/bootstrap.min.css" integrity="sha384-rwoIResjU2yc3z8GV/NPeZWAv56rSmLldC3R/AZzGRnGxQQKnKkoFVhFQhNUwEyJ" crossorigin="anonymous">
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.4/angular.min.js"></script>
<body>
<div class="container text-center mx-auto mt-4 p-4"  style="width: 500px" ng-app="my-App" ng-controller="paintCalculatorController">
	<h3>Room ceiling area</h3>
 	<form class="inline-form mt-3 py-3">
		<label>Length: </label>
		<input style="width:90px" type="number" ng-model="length" value="" name="" min="0" oninput="validity.valid||(value='');">
		<span> feet</span>
		<span class="ml-2 mr-2 font-weight-bold"> X </span>
		<label>Width : </label>
		<input style="width:90px" type="number" ng-model="width" value="" name="" min="0" oninput="validity.valid||(value='');">
		<span> feet</span>
    </form>
	<h4 class="mt-3">You will need to purchase {{countGallon()}} gallons of
	paint to cover {{square()}} square feet.</h4>
</div>
</body>

\$\endgroup\$
1
  • \$\begingroup\$ Simple code is good code. I'm not an angular guy, but I really don't see anything to complain about here besides maybe using $scope.square in $scope.countGallon. \$\endgroup\$
    – Gerrit0
    Mar 14 '18 at 0:28
1
\$\begingroup\$

Feedback

The code looks pretty good, and thus I only found a few things that I could change (listed below). I like how the bootstrap text-center class can be applied once to affect all sub-elements.

Suggestions

CSS

Move the in-line styles to the stylesheet to reduce redundancy - e.g. the number inputs have a specific width, which can be put into a CSS rule-set, like this example:

input[type="number"] {
  width: 90px;
}

Or apply a class name to those elements and utilize that class name in the selector. Also, the width of the container (i.e. 500px) can be moved into a rule-set.

JavaScript

Declaring the Constant

The idiomatic way of declaring a constant in AngularJS is to use .constant() and use dependency injection to inject that constant in the controller.

.constant('SQUARE_FEET_FOR_ONE_GALLON', 350)

Then that value will be available for injection in the controller (this example uses the implicit format:

.controller('paintCalculatorController', function($scope, SQUARE_FEET_FOR_ONE_GALLON){

If nobody else is planning to work on this small amount of code with you, then using a plain JavaScript constant should be fine.

Slight redundancy

Gerrit0 is correct in the comment: "maybe using $scope.square in $scope.countGallon"

The function countGallon() can be updated to updated to utilize the count() function. The formula for the square footage likely won't change anytime soon, but if it did, then that could be updated in a single place instead of both places. This complies with the Don't Repeat Yourself principle.

return Math.ceil($scope.count() / squareFeetForOneGallon);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.