patternjavascriptMinor
Property management
Viewed 0 times
managementpropertystackoverflow
Problem
I was looking to see if anyone could help me come up with a cleaner/DRY methods in duplicate of doing something like this?
I would like to get the features that repeats in the 2 controllers to generalize my code.
```
var propertiesModule = angular.module('app.properties', []);
propertiesModule.controller('PropertiesCtrl', ['$scope', '$routeParams', 'PropertiesService', 'ApplicationService', 'PlatformService', 'Page', function ($scope, $routeParams, PropertiesService, ApplicationService, PlatformService, Page) {
Page.setTitle("Properties");
$scope.platform = $routeParams.platform;
$scope.platforms = [];
$scope.on_edit_platform = function (platform_name) {
/ Reset unit choice /
$scope.unit = undefined;
};
$scope.add_platform = function (platform_name) {
if (!_.contains($scope.platforms, platform_name)) {
$scope.platforms.push(platform_name);
}
};
$scope.delete_platform = function (platform) {
//Might be a bit tricky
};
$scope.on_edit_unit = function (unit) {
PropertiesService.getPropertiesMergedWithModel("properties#" + $scope.application.name + "#" + $scope.application.version + "#" + $scope.platform + "#" + unit.name, unit.modelNamespaces).then(function (properties) {
$scope.properties = properties;
});
};
$scope.save_properties = function (properties) {
PropertiesService.save(properties).then(function (properties) {
PropertiesService.getModel($scope.unit.modelNamespaces).then(function (model) {
$scope.properties = properties.mergeWithModel(model);
});
});
};
$scope.on_concat_plateformes = function () {
var a = $scope.platforms.concat($scope.platformsCible);
for (var i = 0; i b[property]) ? 1 : 0;
return result * sortOrder;
}
}
ApplicationService.get($routeParams.application, $routeParams.version).then(function (appli
I would like to get the features that repeats in the 2 controllers to generalize my code.
```
var propertiesModule = angular.module('app.properties', []);
propertiesModule.controller('PropertiesCtrl', ['$scope', '$routeParams', 'PropertiesService', 'ApplicationService', 'PlatformService', 'Page', function ($scope, $routeParams, PropertiesService, ApplicationService, PlatformService, Page) {
Page.setTitle("Properties");
$scope.platform = $routeParams.platform;
$scope.platforms = [];
$scope.on_edit_platform = function (platform_name) {
/ Reset unit choice /
$scope.unit = undefined;
};
$scope.add_platform = function (platform_name) {
if (!_.contains($scope.platforms, platform_name)) {
$scope.platforms.push(platform_name);
}
};
$scope.delete_platform = function (platform) {
//Might be a bit tricky
};
$scope.on_edit_unit = function (unit) {
PropertiesService.getPropertiesMergedWithModel("properties#" + $scope.application.name + "#" + $scope.application.version + "#" + $scope.platform + "#" + unit.name, unit.modelNamespaces).then(function (properties) {
$scope.properties = properties;
});
};
$scope.save_properties = function (properties) {
PropertiesService.save(properties).then(function (properties) {
PropertiesService.getModel($scope.unit.modelNamespaces).then(function (model) {
$scope.properties = properties.mergeWithModel(model);
});
});
};
$scope.on_concat_plateformes = function () {
var a = $scope.platforms.concat($scope.platformsCible);
for (var i = 0; i b[property]) ? 1 : 0;
return result * sortOrder;
}
}
ApplicationService.get($routeParams.application, $routeParams.version).then(function (appli
Solution
A short, nitpicking review:
-
Here, there seems to be no good reason to choose
could be
-
Also, mandatory bash on using French in variables names. Don't do it,
-
Consider using syntax sugar (in this example using shortcut variables) and remove useless comments, this
could be
-
Be more ardent in reducing copy paste lines, the following seems a good example:
you could try this:
-
You could reduce the vertical stretching in your code quite a bit by using
could be
-
Here, there seems to be no good reason to choose
key and value instead of name and value, it just seems like it makes things more complicated.var tab1 = [];
for (var i = 1; i <= $scope.properties.key_value_properties.length - 1; i++) {
tab1.push({
key: $scope.properties.key_value_properties[i].name,
value: $scope.properties.key_value_properties[i].value
});
}
var tab2 = [];
for (var i = 1; i <= propertiesCible.key_value_properties.length - 1; i++) {
tab2.push({
key: propertiesCible.key_value_properties[i].name,
value: propertiesCible.key_value_properties[i].value
});
}could be
var tab1 = $scope.properties.key_value_properties,
tab2 = propertiesCible.key_value_properties;-
Also, mandatory bash on using French in variables names. Don't do it,
propertiesCible -> propertiesTarget-
Consider using syntax sugar (in this example using shortcut variables) and remove useless comments, this
$scope.properties.differences = [];
$scope.properties.absentInTab1 = [];
$scope.properties.presentInTab1 = [];
for (var i in obj1) {
if (typeof(obj2[i]) === 'undefined') {
// presentInTab1.push({ key: i, oldValue: obj1[i], newValue: null });
$scope.properties.presentInTab1.push({name: i, valueRef: obj1[i], valueCible: null});
}
else if (obj1[i] !== obj2[i]) {
//differences.push({ key: i, oldValue: obj1[i], newValue: obj2[i] });
$scope.properties.differences.push({name: i, valueRef: obj1[i], valueCible: obj2[i]});
}
}
for (var i in obj2) {
if (typeof(obj1[i]) === 'undefined') {
// absentInTab1.push({ key: i, oldValue: null, newValue: obj2[i] });
$scope.properties.absentInTab1.push({name: i, valueRef: null, valueCible: obj2[i]});
}
}could be
var differences = $scope.properties.differences = [],
absentInTab1 = $scope.properties.absentInTab1 = [],
presentInTab1 = $scope.properties.presentInTab1 = [];
for (var i in obj1) {
if (typeof(obj2[i]) === 'undefined') {
presentInTab1.push({name: i, valueRef: obj1[i], valueCible: null});
}
else if (obj1[i] !== obj2[i]) {
differences.push({name: i, valueRef: obj1[i], valueCible: obj2[i]});
}
}
for (var i in obj2) {
if (typeof(obj1[i]) === 'undefined') {
absentInTab1.push({name: i, valueRef: null, valueCible: obj2[i]});
}
}- I strive to never use
nullbutundefinedinstead
-
Be more ardent in reducing copy paste lines, the following seems a good example:
save: function (properties) {
properties = properties.toAppEntity();
if (properties.versionID < 0) {
return $http.post('rest/properties/' + encodeURIComponent(properties.namespace), properties).then(function (response) {
$.notify("Les proprietes ont bien ete crees", "success");
return new Properties(response.data);
}, function (error) {
$.notify(error.data, "error");
});
} else {
return $http.put('rest/properties/' + encodeURIComponent(properties.namespace), properties).then(function (response) {
$.notify("Les proprietes ont bien ete mises a jour", "success");
return new Properties(response.data)
}, function (error) {
$.notify(error.data, "error");
});
}
}you could try this:
save: function (properties) {
var createdMessage = 'Les proprietes ont bien ete crees',
updatedMessage = 'Les proprietes ont bien ete mises a jour',
message = properties.versionID < 0 ? createdMessage : updatedMessage;
properties = properties.toAppEntity();
return $http.put('rest/properties/' + encodeURIComponent(properties.namespace), properties).then(function (response) {
$.notify( message , "success");
return new Properties(response.data)
}, function (error) {
$.notify(error.data, "error");
});
}-
You could reduce the vertical stretching in your code quite a bit by using
_.pick():return {
name: kvp.name,
comment: kvp.comment,
value: kvp.value
}could be
return _.pick( kvp, 'name', 'comment', 'value');Code Snippets
var tab1 = [];
for (var i = 1; i <= $scope.properties.key_value_properties.length - 1; i++) {
tab1.push({
key: $scope.properties.key_value_properties[i].name,
value: $scope.properties.key_value_properties[i].value
});
}
var tab2 = [];
for (var i = 1; i <= propertiesCible.key_value_properties.length - 1; i++) {
tab2.push({
key: propertiesCible.key_value_properties[i].name,
value: propertiesCible.key_value_properties[i].value
});
}var tab1 = $scope.properties.key_value_properties,
tab2 = propertiesCible.key_value_properties;$scope.properties.differences = [];
$scope.properties.absentInTab1 = [];
$scope.properties.presentInTab1 = [];
for (var i in obj1) {
if (typeof(obj2[i]) === 'undefined') {
// presentInTab1.push({ key: i, oldValue: obj1[i], newValue: null });
$scope.properties.presentInTab1.push({name: i, valueRef: obj1[i], valueCible: null});
}
else if (obj1[i] !== obj2[i]) {
//differences.push({ key: i, oldValue: obj1[i], newValue: obj2[i] });
$scope.properties.differences.push({name: i, valueRef: obj1[i], valueCible: obj2[i]});
}
}
for (var i in obj2) {
if (typeof(obj1[i]) === 'undefined') {
// absentInTab1.push({ key: i, oldValue: null, newValue: obj2[i] });
$scope.properties.absentInTab1.push({name: i, valueRef: null, valueCible: obj2[i]});
}
}var differences = $scope.properties.differences = [],
absentInTab1 = $scope.properties.absentInTab1 = [],
presentInTab1 = $scope.properties.presentInTab1 = [];
for (var i in obj1) {
if (typeof(obj2[i]) === 'undefined') {
presentInTab1.push({name: i, valueRef: obj1[i], valueCible: null});
}
else if (obj1[i] !== obj2[i]) {
differences.push({name: i, valueRef: obj1[i], valueCible: obj2[i]});
}
}
for (var i in obj2) {
if (typeof(obj1[i]) === 'undefined') {
absentInTab1.push({name: i, valueRef: null, valueCible: obj2[i]});
}
}save: function (properties) {
properties = properties.toAppEntity();
if (properties.versionID < 0) {
return $http.post('rest/properties/' + encodeURIComponent(properties.namespace), properties).then(function (response) {
$.notify("Les proprietes ont bien ete crees", "success");
return new Properties(response.data);
}, function (error) {
$.notify(error.data, "error");
});
} else {
return $http.put('rest/properties/' + encodeURIComponent(properties.namespace), properties).then(function (response) {
$.notify("Les proprietes ont bien ete mises a jour", "success");
return new Properties(response.data)
}, function (error) {
$.notify(error.data, "error");
});
}
}Context
StackExchange Code Review Q#69744, answer score: 2
Revisions (0)
No revisions yet.