重构Java程序

huangapple
go评论99阅读模式
英文:

Refactor Java Program

问题

  1. List<House>[] arrOfHouseList = getArrOfHouseList();
  2. boolean hasListWithHouseAndCat = false;
  3. boolean hasListWithHouseAndDog = false;
  5. for (List<House> houseList : arrOfHouseList) {
  6.     boolean hasCatOnly = true;
  7.     boolean hasDogOnly = true;
  9.     for (House house : houseList) {
  10.         if (house.hasCat()) {
  11.             hasDogOnly = false;
  12.         } else {
  13.             hasCatOnly = false;
  14.         }
  15.     }
  17.     if (!hasDogOnly && !hasCatOnly) {
  18.         return false; // Mixed case: The list contains both kinds of houses
  19.     }
  21.     hasListWithHouseAndCat |= hasCatOnly;
  22.     hasListWithHouseAndDog |= hasDogOnly;
  23. }
  25. // Check that we have at least one list with all House-Cat and at least one list with all House-Dog
  26. return hasListWithHouseAndCat && hasListWithHouseAndDog;

Here's the refactored code with improved variable naming and reduced the number of boolean variables used. This version uses the |= operator to update the flag variables. This code maintains the same functionality as your original code but is cleaner and more concise.

英文:

Problem: I have list of houses in an Array. I need to make sure atleast one list consist of only houses with cat and atleast one list consist of only houses with dog. A list should only contain one kind of houses (i.e no mixed case). It is assumed that a house can either have a cat or a dog.

I tried writing the code, but it does not look good and would appreciate your help on refactoring or improving the design.

  1.     Boolean isListWithHouseAndCatExist = false;
  2.     Boolean isListWithHouseAndDogExist = false;
  3.     List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
  4.     for(List&lt;House&gt; houseList : arrOfHouseList){
  5.        Boolean isHousesWithCat = true;
  6.        Boolean isHousesWithDog = true;
  7.        for(House house: houseList){
  8.           if(house.hasCat()){
  9.              isHousesWithDog = false;
  10.           }else{
  11.              isHousesWithCat = false;
  12.           }
  13.        }
  14.        if(!isHousesWithDog &amp;&amp; ! isHousesWithCat){
  15.           return false; //This is mixed case. The list contains both of kind of houses
  16.        }
  17.        isListWithHouseAndCatExist=isHousesWithCat?true:isListWithHouseAndCatExist;
  18.        isListWithHouseAndDogExist=isHousesWithDog?true:isListWithHouseAndDogExist;  
  19.     }
  21.     // Now to check that we have atleast one list with all House-Cat and atleast one list with all
  22.     // House-Dog
  23.     if(!isListWithHouseAndCatExist || !isListWithHouseAndDogExist){
  24.       return false;
  25.    }
  26.    return true;

As you can see I had to use four Boolean variable to validate conditions. Could you please help to improve the code.

答案1

得分: 3

如果您不想使用流（streams）而只想迭代一次，您可以像这样做：

  1. Boolean isListWithHouseAndCatExist = false;
  2. Boolean isListWithHouseAndDogExist = false;
  3. List<House>[] arrOfHouseList = getArrOfHouseList();
  4. for (List<House> houseList : arrOfHouseList) {
  6.     Set<Boolean> hasCatFlags = new HashSet<>();
  7.     for (House house : houseList) {
  8.         hasCatFlags.add(house.hasCat());
  9.     }
  10.     if (hasCatFlags.size() > 1) {
  11.         return false; // 这是混合情况。列表包含两种类型的房屋
  12.     }
  13.     if (hasCatFlags.contains(true)) {
  14.         isListWithHouseAndCatExist = true;
  15.     } else if (hasCatFlags.contains(false)) {
  16.         isListWithHouseAndDogExist = true;
  17.     }
  18. }
  20. return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

如果您可以使用流（streams）但只想迭代一次，您可以像这样做：

  1. Boolean isListWithHouseAndCatExist = false;
  2. Boolean isListWithHouseAndDogExist = false;
  3. List<House>[] arrOfHouseList = getArrOfHouseList();
  4. for (List<House> houseList : arrOfHouseList) {
  6.     Set<Boolean> hasCatFlags = houseList.stream().map(House::hasCat).collect(Collectors.toSet());
  7.     if (hasCatFlags.size() > 1) {
  8.         return false; // 这是混合情况。列表包含两种类型的房屋
  9.     }
  11.     if (hasCatFlags.contains(true)) {
  12.         isListWithHouseAndCatExist = true;
  13.     } else if (hasCatFlags.contains(false)) {
  14.         isListWithHouseAndDogExist = true;
  15.     }
  16. }
  18. return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

如果您可以使用流（streams）并且不介意迭代两次，您可以像这样做：

  1. Boolean isListWithHouseAndCatExist = false;
  2. Boolean isListWithHouseAndDogExist = false;
  3. for (List<House> houseList : getArrOfHouseList()) {
  4.     if (houseList.stream().allMatch(House::hasCat)) {
  5.         isListWithHouseAndCatExist = true;
  6.     } else if (houseList.stream().noneMatch(House::hasCat)) {
  7.         isListWithHouseAndDogExist = true;
  8.     } else {
  9.         return false; // 混合情况
  10.     }
  11. }
  13. return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

希望这能帮助您！

英文:

If you don't want to use streams and only want to iterate once, you could do something like this:

  1.     Boolean isListWithHouseAndCatExist = false;
  2.     Boolean isListWithHouseAndDogExist = false;
  3.     List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
  4.     for(List&lt;House&gt; houseList : arrOfHouseList){
  6.         Set&lt;Boolean&gt; hasCatFlags = new HashSet&lt;&gt;();
  7.         for(House house: houseList){
  8.             hasCatFlags.add(house.hasCat());
  9.         }
  10.         if(hasCatFlags.size() &gt; 1){
  11.             return false; //This is mixed case. The list contains both of kind of houses
  12.         }
  13.         if (hasCatFlags.contains(true)) {
  14.             isListWithHouseAndCatExist = true;
  15.         } else if (hasCatFlags.contains(false)) {
  16.             isListWithHouseAndDogExist = true;
  17.         }
  18.     }
  20.     return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

If you can use streams but want to iterate only once, you could do something like this:

  1.     Boolean isListWithHouseAndCatExist = false;
  2.     Boolean isListWithHouseAndDogExist = false;
  3.     List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
  4.     for(List&lt;House&gt; houseList : arrOfHouseList){
  6.         Set&lt;Boolean&gt; hasCatFlags = houseList.stream().map(House::hasCat).collect(Collectors.toSet());
  7.         if(hasCatFlags.size() &gt; 1){
  8.             return false; //This is mixed case. The list contains both of kind of houses
  9.         }
  10.         
  11.         if (hasCatFlags.contains(true)) {
  12.             isListWithHouseAndCatExist = true;
  13.         } else if (hasCatFlags.contains(false)) {
  14.             isListWithHouseAndDogExist = true;
  15.         }
  16.     }
  18.     return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

And if you can use streams and don't mind iterating twice, you could do this:

  1.     Boolean isListWithHouseAndCatExist = false;
  2.     Boolean isListWithHouseAndDogExist = false;
  3.     for(List&lt;House&gt; houseList : getArrOfHouseList()){
  4.         if (houseList.stream().allMatch(House::hasCat)) {
  5.             isListWithHouseAndCatExist = true;
  6.         } else if (houseList.stream().noneMatch(House::hasCat)) {
  7.             isListWithHouseAndDogExist = true;
  8.         } else {
  9.             return false;//mixed case
  10.         } 
  11.     }
  13.     return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

huangapple
  • 本文由 发表于 2020年5月29日 12:37:06
  • 转载请务必保留本文链接：https://go.coder-hub.com/62078830.html
  • java
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定