2
\$\begingroup\$

I have created a Laravel Single Page Application using Angular Js 1.3.14 by learning tutorials in the websites.

index.php:

 <body  ng-app = "mainApp">
   <nav class="navbar navbar-default navbar-static-top" style="background-
   color:orange;">
     <div class="container">
       <div class="navbar-header">
         <a class="navbar-brand prname" href="#home" 
         style="color:red!important;font-weight:bold;font-size:20px;">
         Company Name
         </a>
       </div>
       <div class="collapse navbar-collapse" id="app-navbar-collapse">
         <ul class="nav navbar-nav">
           <li class="menustyle home" style="background-color: red;"><a 
           href="#home">Home</a></li>
           <li class="menustyle about"><a href="#aboutus">About Us</a></li>
           </li>
         </ul> 
       </div>
     </div>
   </nav>
    <div class="flex-center position-ref full-height"  style="margin-top:-100px;">

         <div ng-view></div>  

    </div>
    <script src = "{{ asset('js/main.js') }}"></script>
    <script src = "{{ asset('js/controller.js') }}"></script>
  </body>

web.php:

Route::get('/', function () {
   return view('welcome');
});
Route::get('/home', function () {
   return view('home');
});
Route::get('/aboutus', function () {
    return view('aboutus');
});
Route::get('getusers', 'UserController@users');

UserController.php:

class UserController extends Controller
{
    public function users(Request $request)
    {
        $users = DB::table('users')->get();
        return response()->json($users);
    }
}

aboutus.blade.php:

<div ng-controller="AddStudentController">
<table class="table table-bordered">
  <thead>
    <tr>
      <th>Id</th>
      <th>Name</th>
      <th>Email</th>
      <th>Created At</th>
    </tr>
  </thead>
  <tbody>
    <tr ng-repeat = "data in datas">
      <td>@{{ data.id }}</td>
      <td>@{{ data.name }}</td>
      <td>@{{ data.email }}</td>
      <td>@{{ data.created_at }}</td>
    </tr>
  </tbody>
</table>

controller.js:

 var mainApp = angular.module("mainApp", ['ngRoute']);
 mainApp.config(['$routeProvider', function($routeProvider) {
    $routeProvider.

    when('/home', {
       templateUrl: '/home',
    }).

    when('/aboutus', {
       templateUrl: '/aboutus',
       controller: 'AddStudentController'
    }).

    when('/services', {
       templateUrl: '/services',
    }).
 }]);    

  mainApp.controller('AddStudentController', function($scope, $http) {
    $http.get('getusers').then( function(response) {
      $scope.datas = response.data;
    });
  });

The SPA which i have created working correctly, i have developed this project of my own. Is there any thing i want to change or any mistake in routing or any thing in this project.

\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

This code seems okay for the most part - I don't see any major flaws with the code, just a few things that I will critique below.

Despite being spread across quite a few files, this code seems pretty simple. Not that it is by any means wrong, but it is a bit unique to see AngularJS code paired with Laravel code.

While Laravel does not dictate which JavaScript or CSS pre-processors you use, it does provide a basic starting point using Bootstrap and Vue that will be helpful for many applications.1

The biggest thing I would critique is the lack of request failure handling. While it may be very unlikely that the AJAX requests to get the list of users would fail, due to the simplicity of the controller method and the fact that if that fails it might also be the case that the page wouldn't have loaded in the first place, it is wise to handle such cases gracefully. Typically when calling .then() on a Promise, the second argument should be supplied for the case when the promise is rejected.

$http.get('getusers').then( function(response) {
  $scope.datas = response.data;
}, 
function(rejectionReason) {
  //handle case when request fails here
});

Or instead of passing the rejection handler as a second argument, it could be passed in a subsequent call to .catch()

Other than that, I would bring up the naming of $scope.datas in the controller:

$http.get('getusers').then( function(response) {
  $scope.datas = response.data;
});

datas is a very generic term. Somebody reading that response could think "After getting the response from getusers, Do we need to abstract the users from response.data? A more appropriate name might just be users:

$http.get('getusers').then( function(response) {
  $scope.users = response.data;
});

Then the markup can be updated accordingly:

 <tr ng-repeat="user in users">
   <td>@{{ user.id }}</td>
   <td>@{{ user.name }}</td>
   <td>@{{ user.email }}</td>
   <td>@{{ user.created_at }}</td>
 </tr>

Other than that, I would suggest conditionally showing the table - only if there are entries in the data array. For that, ng-if and/or ng-show can be used. See the snippet below for an example

var mainApp = angular.module("mainApp", []);
mainApp.controller('AddStudentController', function($scope, $http) {
  $scope.requestSent = false;
  updateUsers = function() {
    $http.get('https://jsonplaceholder.typicode.com/users').then(function(response) {
      $scope.users = response.data;
    }, function(rejectionReason) {
      //display error message?
      $scope.error = rejectionReason;
    })
    .finally(function() {
      $scope.requestSent = true;
    });
  };
  setTimeout(updateUsers, 2000); //wait 2 seconds to simulate network latency
});
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.3.14/angular.min.js"></script>
<div ng-app="mainApp">
 <nav class="navbar navbar-default navbar-static-top" style="background-
   color:orange;">
     <div class="container">
       <div class="navbar-header">
         <a class="navbar-brand prname" href="#home" 
         style="color:red!important;font-weight:bold;font-size:20px;">
         Company Name
         </a>
       </div>
       <div class="collapse navbar-collapse" id="app-navbar-collapse">
         <ul class="nav navbar-nav">
           <li class="menustyle home" style="background-color: red;"><a 
           href="#home">Home</a></li>
           <li class="menustyle about"><a href="#aboutus">About Us</a></li>
           </li>
         </ul> 
       </div>
     </div>
   </nav>
    <div class="flex-center position-ref full-height" >

         <div ng-view></div>  

    </div>
  <div ng-controller="AddStudentController">
    <img src="https://stackoverflow.com/content/img/progress-dots.gif" ng-hide="requestSent"/>
    <table class="table table-bordered" ng-if="users.length" >
      <thead>
        <tr>
          <th>Id</th>
          <th>Name</th>
          <th>Email</th>
          <th>Created At</th>
        </tr>
      </thead>
      <tbody>
        <tr ng-repeat="user in users">
          <td>@{{ user.id }}</td>
          <td>@{{ user.name }}</td>
          <td>@{{ user.email }}</td>
          <td>@{{ user.created_at }}</td>
        </tr>
      </tbody>
    </table>
    <div ng-bind="error"></div>
  </div>

1https://laravel.com/docs/5.7/frontend

\$\endgroup\$
2
  • \$\begingroup\$ Ok Sam, what I am doing is right or wrong? \$\endgroup\$
    – Kannan K
    Commented Oct 11, 2018 at 17:57
  • 1
    \$\begingroup\$ Yeah so far it seems what you are doing is right... I don't see much for bad practices other than the minor things I pointed out above... \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Commented Oct 11, 2018 at 18:17

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