Comparing JSON documents for changes to values
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
2
down vote
favorite
Please go through below code and improve the quality by reducing the IF conditions:
I have 2 jsons, first one is the source and second one is the updated json. I have to compare both values and set true/false flags. Based on the flags i need to proceed further.
I am checking below conditions:
- NULL changed to DECLINED
- NULL changed to a VALUE
- VALUE changed to DECLINED
- VALUE1 changed to VALUE2
O/P -
canProceed = (nullDec) ? (nullDec && (nullVal || valDec || valVal)) :(nullVal || valDec || valVal);
Plnkr - https://plnkr.co/edit/gKJXibTFQhInBOhz?open=lib%2Fscript.js
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
javascript angular.js null
add a comment |Â
up vote
2
down vote
favorite
Please go through below code and improve the quality by reducing the IF conditions:
I have 2 jsons, first one is the source and second one is the updated json. I have to compare both values and set true/false flags. Based on the flags i need to proceed further.
I am checking below conditions:
- NULL changed to DECLINED
- NULL changed to a VALUE
- VALUE changed to DECLINED
- VALUE1 changed to VALUE2
O/P -
canProceed = (nullDec) ? (nullDec && (nullVal || valDec || valVal)) :(nullVal || valDec || valVal);
Plnkr - https://plnkr.co/edit/gKJXibTFQhInBOhz?open=lib%2Fscript.js
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
javascript angular.js null
add a comment |Â
up vote
2
down vote
favorite
up vote
2
down vote
favorite
Please go through below code and improve the quality by reducing the IF conditions:
I have 2 jsons, first one is the source and second one is the updated json. I have to compare both values and set true/false flags. Based on the flags i need to proceed further.
I am checking below conditions:
- NULL changed to DECLINED
- NULL changed to a VALUE
- VALUE changed to DECLINED
- VALUE1 changed to VALUE2
O/P -
canProceed = (nullDec) ? (nullDec && (nullVal || valDec || valVal)) :(nullVal || valDec || valVal);
Plnkr - https://plnkr.co/edit/gKJXibTFQhInBOhz?open=lib%2Fscript.js
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
javascript angular.js null
Please go through below code and improve the quality by reducing the IF conditions:
I have 2 jsons, first one is the source and second one is the updated json. I have to compare both values and set true/false flags. Based on the flags i need to proceed further.
I am checking below conditions:
- NULL changed to DECLINED
- NULL changed to a VALUE
- VALUE changed to DECLINED
- VALUE1 changed to VALUE2
O/P -
canProceed = (nullDec) ? (nullDec && (nullVal || valDec || valVal)) :(nullVal || valDec || valVal);
Plnkr - https://plnkr.co/edit/gKJXibTFQhInBOhz?open=lib%2Fscript.js
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
import angular from 'angular';
angular.module('plunker', ).controller('MainCtrl', function($scope)
$scope.name = 'Plunker';
var json1=id:"01", name:"Jon", city:null, state:"NY", phone:null, zip:"06033";
var json2=id:"02", name:"Kris", city:"DECLINED", state:"DECLINED", phone:"860-000-0000", zip:"06033";
var json3=id:"02", name:"Sam", city:"EDISON", state:"DECLINED", phone:"860-000-0000", zip:"06033";
compareJsons(json1, json2);
function compareJsons(model1, model2) valDec
);
javascript angular.js null
edited May 1 at 3:43
200_success
123k14142399
123k14142399
asked May 1 at 3:25
asder
133
133
add a comment |Â
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
0
down vote
accepted
Your code look good, but there is some room for improvements.
Comment your code
Its is a good pratice to comment the behavior that you code should have.
There is a good article written by Jeff Atwood called "Code Tells You How, Comments Tell You Why",
when you have time you should read it, but the best way that i find to write comments its to write them
before the code itself.
So if i had to write your code myself my first step would be something like:
// iterate all properties
// check if the the first value is defined
// ...
// if the value is not defined then
// ...
The hard part its (almost) never writing the code itself but thinking about it, if you have no problems
writing these comments you should have no problems writing the code itself. This helps you to make sure that
you undertand the problem that you are trying to solve with code, if If you find diffucults in this step you
should spend more time thinking about the problem.
Iterators
You are using forEach
but seems that you only need to iterate until the first positive result so you may use some
,
so your function will not need to iterate all the elements, so your function would be something like:
angular.some(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
console.log(m1Key +" - " + m1Val + " CHANGED TO " + m2Key +" - " + m2Val);
if((m2Val === decline) && !(m1Val))
nullDec = true;
if(((m2Val) && (m2Val !== decline)) && !(m1Val))
nullVal = true;
return nullDec && nullVal;
)
Conditionals
Inside the if(!m1Val)
you have the conditionals (m2Val === decline) && !(m1Val)
and (m2Val) && (m2Val !== decline)) && !(m1Val)
,
you don't need to check m1Val
again because its was already checked, the same its true to the block of code of your else statment.
Complexity
If you are sure that all the keys in model1 is always exist in model2 you don't need to iterate model2 to get its value.
Instead of the second forEach if the (m2Key === m1Key)
you can get the value of the m1Key
in the model2
object without the second loop;
This code have O(n^2) complexity:
angular.forEach(model1, function(m1Val, m1Key)
if(!m1Val)
angular.forEach(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
)
This one below have O(n), but will need to make sure that all model1 and model2 have the same keys, or add some conditionals to
make sure that the behavior is correct.
angular.forEach(model1, function(m1Val, m1Key)
if (!m1Val)
if (m1Val !== model2[m1Key])
// ...
Even without these improvements your code passed all the test cases that i could come up with, so you just need to adjust your
style and get deep understanding of the JS api. I can see that you tried to cover all possible cases that is pretty good, you should continue aim for this as much as possible.
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,m2Val
is always equals tomodel2[m1key]
, besides that the code should be good to go.
â Josenberg
May 3 at 17:13
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
accepted
Your code look good, but there is some room for improvements.
Comment your code
Its is a good pratice to comment the behavior that you code should have.
There is a good article written by Jeff Atwood called "Code Tells You How, Comments Tell You Why",
when you have time you should read it, but the best way that i find to write comments its to write them
before the code itself.
So if i had to write your code myself my first step would be something like:
// iterate all properties
// check if the the first value is defined
// ...
// if the value is not defined then
// ...
The hard part its (almost) never writing the code itself but thinking about it, if you have no problems
writing these comments you should have no problems writing the code itself. This helps you to make sure that
you undertand the problem that you are trying to solve with code, if If you find diffucults in this step you
should spend more time thinking about the problem.
Iterators
You are using forEach
but seems that you only need to iterate until the first positive result so you may use some
,
so your function will not need to iterate all the elements, so your function would be something like:
angular.some(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
console.log(m1Key +" - " + m1Val + " CHANGED TO " + m2Key +" - " + m2Val);
if((m2Val === decline) && !(m1Val))
nullDec = true;
if(((m2Val) && (m2Val !== decline)) && !(m1Val))
nullVal = true;
return nullDec && nullVal;
)
Conditionals
Inside the if(!m1Val)
you have the conditionals (m2Val === decline) && !(m1Val)
and (m2Val) && (m2Val !== decline)) && !(m1Val)
,
you don't need to check m1Val
again because its was already checked, the same its true to the block of code of your else statment.
Complexity
If you are sure that all the keys in model1 is always exist in model2 you don't need to iterate model2 to get its value.
Instead of the second forEach if the (m2Key === m1Key)
you can get the value of the m1Key
in the model2
object without the second loop;
This code have O(n^2) complexity:
angular.forEach(model1, function(m1Val, m1Key)
if(!m1Val)
angular.forEach(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
)
This one below have O(n), but will need to make sure that all model1 and model2 have the same keys, or add some conditionals to
make sure that the behavior is correct.
angular.forEach(model1, function(m1Val, m1Key)
if (!m1Val)
if (m1Val !== model2[m1Key])
// ...
Even without these improvements your code passed all the test cases that i could come up with, so you just need to adjust your
style and get deep understanding of the JS api. I can see that you tried to cover all possible cases that is pretty good, you should continue aim for this as much as possible.
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,m2Val
is always equals tomodel2[m1key]
, besides that the code should be good to go.
â Josenberg
May 3 at 17:13
add a comment |Â
up vote
0
down vote
accepted
Your code look good, but there is some room for improvements.
Comment your code
Its is a good pratice to comment the behavior that you code should have.
There is a good article written by Jeff Atwood called "Code Tells You How, Comments Tell You Why",
when you have time you should read it, but the best way that i find to write comments its to write them
before the code itself.
So if i had to write your code myself my first step would be something like:
// iterate all properties
// check if the the first value is defined
// ...
// if the value is not defined then
// ...
The hard part its (almost) never writing the code itself but thinking about it, if you have no problems
writing these comments you should have no problems writing the code itself. This helps you to make sure that
you undertand the problem that you are trying to solve with code, if If you find diffucults in this step you
should spend more time thinking about the problem.
Iterators
You are using forEach
but seems that you only need to iterate until the first positive result so you may use some
,
so your function will not need to iterate all the elements, so your function would be something like:
angular.some(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
console.log(m1Key +" - " + m1Val + " CHANGED TO " + m2Key +" - " + m2Val);
if((m2Val === decline) && !(m1Val))
nullDec = true;
if(((m2Val) && (m2Val !== decline)) && !(m1Val))
nullVal = true;
return nullDec && nullVal;
)
Conditionals
Inside the if(!m1Val)
you have the conditionals (m2Val === decline) && !(m1Val)
and (m2Val) && (m2Val !== decline)) && !(m1Val)
,
you don't need to check m1Val
again because its was already checked, the same its true to the block of code of your else statment.
Complexity
If you are sure that all the keys in model1 is always exist in model2 you don't need to iterate model2 to get its value.
Instead of the second forEach if the (m2Key === m1Key)
you can get the value of the m1Key
in the model2
object without the second loop;
This code have O(n^2) complexity:
angular.forEach(model1, function(m1Val, m1Key)
if(!m1Val)
angular.forEach(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
)
This one below have O(n), but will need to make sure that all model1 and model2 have the same keys, or add some conditionals to
make sure that the behavior is correct.
angular.forEach(model1, function(m1Val, m1Key)
if (!m1Val)
if (m1Val !== model2[m1Key])
// ...
Even without these improvements your code passed all the test cases that i could come up with, so you just need to adjust your
style and get deep understanding of the JS api. I can see that you tried to cover all possible cases that is pretty good, you should continue aim for this as much as possible.
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,m2Val
is always equals tomodel2[m1key]
, besides that the code should be good to go.
â Josenberg
May 3 at 17:13
add a comment |Â
up vote
0
down vote
accepted
up vote
0
down vote
accepted
Your code look good, but there is some room for improvements.
Comment your code
Its is a good pratice to comment the behavior that you code should have.
There is a good article written by Jeff Atwood called "Code Tells You How, Comments Tell You Why",
when you have time you should read it, but the best way that i find to write comments its to write them
before the code itself.
So if i had to write your code myself my first step would be something like:
// iterate all properties
// check if the the first value is defined
// ...
// if the value is not defined then
// ...
The hard part its (almost) never writing the code itself but thinking about it, if you have no problems
writing these comments you should have no problems writing the code itself. This helps you to make sure that
you undertand the problem that you are trying to solve with code, if If you find diffucults in this step you
should spend more time thinking about the problem.
Iterators
You are using forEach
but seems that you only need to iterate until the first positive result so you may use some
,
so your function will not need to iterate all the elements, so your function would be something like:
angular.some(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
console.log(m1Key +" - " + m1Val + " CHANGED TO " + m2Key +" - " + m2Val);
if((m2Val === decline) && !(m1Val))
nullDec = true;
if(((m2Val) && (m2Val !== decline)) && !(m1Val))
nullVal = true;
return nullDec && nullVal;
)
Conditionals
Inside the if(!m1Val)
you have the conditionals (m2Val === decline) && !(m1Val)
and (m2Val) && (m2Val !== decline)) && !(m1Val)
,
you don't need to check m1Val
again because its was already checked, the same its true to the block of code of your else statment.
Complexity
If you are sure that all the keys in model1 is always exist in model2 you don't need to iterate model2 to get its value.
Instead of the second forEach if the (m2Key === m1Key)
you can get the value of the m1Key
in the model2
object without the second loop;
This code have O(n^2) complexity:
angular.forEach(model1, function(m1Val, m1Key)
if(!m1Val)
angular.forEach(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
)
This one below have O(n), but will need to make sure that all model1 and model2 have the same keys, or add some conditionals to
make sure that the behavior is correct.
angular.forEach(model1, function(m1Val, m1Key)
if (!m1Val)
if (m1Val !== model2[m1Key])
// ...
Even without these improvements your code passed all the test cases that i could come up with, so you just need to adjust your
style and get deep understanding of the JS api. I can see that you tried to cover all possible cases that is pretty good, you should continue aim for this as much as possible.
Your code look good, but there is some room for improvements.
Comment your code
Its is a good pratice to comment the behavior that you code should have.
There is a good article written by Jeff Atwood called "Code Tells You How, Comments Tell You Why",
when you have time you should read it, but the best way that i find to write comments its to write them
before the code itself.
So if i had to write your code myself my first step would be something like:
// iterate all properties
// check if the the first value is defined
// ...
// if the value is not defined then
// ...
The hard part its (almost) never writing the code itself but thinking about it, if you have no problems
writing these comments you should have no problems writing the code itself. This helps you to make sure that
you undertand the problem that you are trying to solve with code, if If you find diffucults in this step you
should spend more time thinking about the problem.
Iterators
You are using forEach
but seems that you only need to iterate until the first positive result so you may use some
,
so your function will not need to iterate all the elements, so your function would be something like:
angular.some(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
console.log(m1Key +" - " + m1Val + " CHANGED TO " + m2Key +" - " + m2Val);
if((m2Val === decline) && !(m1Val))
nullDec = true;
if(((m2Val) && (m2Val !== decline)) && !(m1Val))
nullVal = true;
return nullDec && nullVal;
)
Conditionals
Inside the if(!m1Val)
you have the conditionals (m2Val === decline) && !(m1Val)
and (m2Val) && (m2Val !== decline)) && !(m1Val)
,
you don't need to check m1Val
again because its was already checked, the same its true to the block of code of your else statment.
Complexity
If you are sure that all the keys in model1 is always exist in model2 you don't need to iterate model2 to get its value.
Instead of the second forEach if the (m2Key === m1Key)
you can get the value of the m1Key
in the model2
object without the second loop;
This code have O(n^2) complexity:
angular.forEach(model1, function(m1Val, m1Key)
if(!m1Val)
angular.forEach(model2, function(m2Val, m2Key)
if((m2Key === m1Key) && (m2Val !== m1Val))
)
This one below have O(n), but will need to make sure that all model1 and model2 have the same keys, or add some conditionals to
make sure that the behavior is correct.
angular.forEach(model1, function(m1Val, m1Key)
if (!m1Val)
if (m1Val !== model2[m1Key])
// ...
Even without these improvements your code passed all the test cases that i could come up with, so you just need to adjust your
style and get deep understanding of the JS api. I can see that you tried to cover all possible cases that is pretty good, you should continue aim for this as much as possible.
answered May 2 at 12:45
Josenberg
43827
43827
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,m2Val
is always equals tomodel2[m1key]
, besides that the code should be good to go.
â Josenberg
May 3 at 17:13
add a comment |Â
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,m2Val
is always equals tomodel2[m1key]
, besides that the code should be good to go.
â Josenberg
May 3 at 17:13
1
1
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Hi appreciate your feedback.. i have modified code like below plnkr.co/edit/mdttacWm4I1xDHXM?open=lib%2Fscript.js
â asder
May 3 at 3:32
Inside your loop,
m2Val
is always equals to model2[m1key]
, besides that the code should be good to go.â Josenberg
May 3 at 17:13
Inside your loop,
m2Val
is always equals to model2[m1key]
, besides that the code should be good to go.â Josenberg
May 3 at 17:13
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f193320%2fcomparing-json-documents-for-changes-to-values%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password