Comparing JSON documents for changes to values

The name of the pictureThe name of the pictureThe name of the pictureClash 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:



  1. NULL changed to DECLINED

  2. NULL changed to a VALUE

  3. VALUE changed to DECLINED

  4. 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

);









share|improve this question



























    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:



    1. NULL changed to DECLINED

    2. NULL changed to a VALUE

    3. VALUE changed to DECLINED

    4. 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

    );









    share|improve this question























      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:



      1. NULL changed to DECLINED

      2. NULL changed to a VALUE

      3. VALUE changed to DECLINED

      4. 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

      );









      share|improve this question













      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:



      1. NULL changed to DECLINED

      2. NULL changed to a VALUE

      3. VALUE changed to DECLINED

      4. 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

      );








      share|improve this question












      share|improve this question




      share|improve this question








      edited May 1 at 3:43









      200_success

      123k14142399




      123k14142399









      asked May 1 at 3:25









      asder

      133




      133




















          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.






          share|improve this answer

















          • 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 to model2[m1key], besides that the code should be good to go.
            – Josenberg
            May 3 at 17:13










          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          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






























          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.






          share|improve this answer

















          • 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 to model2[m1key], besides that the code should be good to go.
            – Josenberg
            May 3 at 17:13














          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.






          share|improve this answer

















          • 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 to model2[m1key], besides that the code should be good to go.
            – Josenberg
            May 3 at 17:13












          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.






          share|improve this answer













          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.







          share|improve this answer













          share|improve this answer



          share|improve this answer











          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 to model2[m1key], besides that the code should be good to go.
            – Josenberg
            May 3 at 17:13












          • 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 to model2[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












           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          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













































































          Popular posts from this blog

          Greedy Best First Search implementation in Rust

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          C++11 CLH Lock Implementation