HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

English-Russian irregular words tester

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
englishwordsirregulartesterrussian

Problem

Could you please offer some suggestions about my self learning project?

It is really hard-coded, but I have no idea what to do next. I'm using angular (only few features for now) and grunt + bower.

tester.html


  
    First form
    Second form
    Third form
    Translation
  
  

    
      
        {{ verb }}
        
      
    

    
      
        {{ verb }}
        
          
          
          {{ val[$index] }}
        
      
    

    
      
        {{ verb }}
      
    

  

  Check

  Next


iwt.coffee

```
angular.module('IWTTester', [])

.controller('IWTTesterController', [
'$http'
'$scope'
($http, $scope) ->

$http.get 'js/words.json'
.success (data, status, headers, config) ->
console.log data, status, config, 'good'
$scope.irregulars = data
$scope.init()
.error (data, status, headers, config) ->
console.log data, status, config, 'bad'

$scope.isActive = (formIndex) ->
formIndex is $scope._activeForm

$scope.isStage = (value) ->
value is $scope._stage

$scope.init = ->
ROWS = $scope.irregulars.length
COLS = $scope.irregulars[0].length

create_counter = (module) ->
i = 0
-> (i++) % module
rowCounter = create_counter(ROWS)
colCounter = create_counter(COLS)

$scope.val = []
$scope._stage = 'input'

update = ->
$scope.verbs = $scope.irregulars[rowCounter()]
$scope._activeForm = colCounter()
update()

$scope.isSuccess = (i) ->
$scope.val[i] is $scope.verbs[i]

$scope.isDanger = (i) ->
$scope.val[i] isnt $scope.verbs[i] and not $scope.isActive(i)

$scope.check = ->
$scope._stage = 'validation'

$scope.next = ->
$scope.val = []
update()
$scope._stage = 'input'
])

.directive('iwtTester', ->
restrict: 'AE'
templateUrl: 'templates

Solution

First of all, I would architecture the data in a more readable and flexible way:

[
  {
    infinitive: "be",
    past: "was",
    participle: "been",
    russian: "быть"
  },
 {
   ...
 },
...
]


That tells me as reader a lot and leaves vast room for future changes without breaking the rest of your code.

Further, if you want to make your code readable for others (or even for yourself after a while), I would recommend to use descriptive names for your functions and variables. When I read



I get no clue what the function isStage does from its name.
So I am forced to scan the rest of your code for its definition, which costs time. You could help me by giving it a name that would give me a rough idea without the need to go elsewhere.

Another thing I see is the use of $http inside controller, which is an anti-pattern. It sadly appears in many tutorials, yet it breaks one of the most fundamental principles of clean code - the single responsiblity principle. Your controller is responsible for gluing your scope (ViewModel layer) with your View, but not for pulling data from your Model layer. That is the job of a dedicated Service.

Code Snippets

[
  {
    infinitive: "be",
    past: "was",
    participle: "been",
    russian: "быть"
  },
 {
   ...
 },
...
]
<tr ng-show="isStage('input')">

Context

StackExchange Code Review Q#64070, answer score: 2

Revisions (0)

No revisions yet.