重构Java程序

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

Refactor Java Program

问题

List<House>[] arrOfHouseList = getArrOfHouseList();
boolean hasListWithHouseAndCat = false;
boolean hasListWithHouseAndDog = false;

for (List<House> houseList : arrOfHouseList) {
    boolean hasCatOnly = true;
    boolean hasDogOnly = true;

    for (House house : houseList) {
        if (house.hasCat()) {
            hasDogOnly = false;
        } else {
            hasCatOnly = false;
        }
    }

    if (!hasDogOnly && !hasCatOnly) {
        return false; // Mixed case: The list contains both kinds of houses
    }

    hasListWithHouseAndCat |= hasCatOnly;
    hasListWithHouseAndDog |= hasDogOnly;
}

// Check that we have at least one list with all House-Cat and at least one list with all House-Dog
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.

    Boolean isListWithHouseAndCatExist = false;
    Boolean isListWithHouseAndDogExist = false;
    List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
    for(List&lt;House&gt; houseList : arrOfHouseList){
       Boolean isHousesWithCat = true;
       Boolean isHousesWithDog = true;
       for(House house: houseList){
          if(house.hasCat()){
             isHousesWithDog = false;
          }else{
             isHousesWithCat = false;
          }
       }
       if(!isHousesWithDog &amp;&amp; ! isHousesWithCat){
          return false; //This is mixed case. The list contains both of kind of houses
       }
       isListWithHouseAndCatExist=isHousesWithCat?true:isListWithHouseAndCatExist;
       isListWithHouseAndDogExist=isHousesWithDog?true:isListWithHouseAndDogExist;  
    }

    // Now to check that we have atleast one list with all House-Cat and atleast one list with all
    // House-Dog
    if(!isListWithHouseAndCatExist || !isListWithHouseAndDogExist){
      return false;
   }
   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)而只想迭代一次,您可以像这样做:

Boolean isListWithHouseAndCatExist = false;
Boolean isListWithHouseAndDogExist = false;
List<House>[] arrOfHouseList = getArrOfHouseList();
for (List<House> houseList : arrOfHouseList) {

    Set<Boolean> hasCatFlags = new HashSet<>();
    for (House house : houseList) {
        hasCatFlags.add(house.hasCat());
    }
    if (hasCatFlags.size() > 1) {
        return false; // 这是混合情况。列表包含两种类型的房屋
    }
    if (hasCatFlags.contains(true)) {
        isListWithHouseAndCatExist = true;
    } else if (hasCatFlags.contains(false)) {
        isListWithHouseAndDogExist = true;
    }
}

return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

如果您可以使用流(streams)但只想迭代一次,您可以像这样做:

Boolean isListWithHouseAndCatExist = false;
Boolean isListWithHouseAndDogExist = false;
List<House>[] arrOfHouseList = getArrOfHouseList();
for (List<House> houseList : arrOfHouseList) {

    Set<Boolean> hasCatFlags = houseList.stream().map(House::hasCat).collect(Collectors.toSet());
    if (hasCatFlags.size() > 1) {
        return false; // 这是混合情况。列表包含两种类型的房屋
    }

    if (hasCatFlags.contains(true)) {
        isListWithHouseAndCatExist = true;
    } else if (hasCatFlags.contains(false)) {
        isListWithHouseAndDogExist = true;
    }
}

return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

如果您可以使用流(streams)并且不介意迭代两次,您可以像这样做:

Boolean isListWithHouseAndCatExist = false;
Boolean isListWithHouseAndDogExist = false;
for (List<House> houseList : getArrOfHouseList()) {
    if (houseList.stream().allMatch(House::hasCat)) {
        isListWithHouseAndCatExist = true;
    } else if (houseList.stream().noneMatch(House::hasCat)) {
        isListWithHouseAndDogExist = true;
    } else {
        return false; // 混合情况
    }
}

return isListWithHouseAndCatExist && isListWithHouseAndDogExist;

希望这能帮助您!

英文:

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

    Boolean isListWithHouseAndCatExist = false;
    Boolean isListWithHouseAndDogExist = false;
    List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
    for(List&lt;House&gt; houseList : arrOfHouseList){

        Set&lt;Boolean&gt; hasCatFlags = new HashSet&lt;&gt;();
        for(House house: houseList){
            hasCatFlags.add(house.hasCat());
        }
        if(hasCatFlags.size() &gt; 1){
            return false; //This is mixed case. The list contains both of kind of houses
        }
        if (hasCatFlags.contains(true)) {
            isListWithHouseAndCatExist = true;
        } else if (hasCatFlags.contains(false)) {
            isListWithHouseAndDogExist = true;
        }
    }

    return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

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

    Boolean isListWithHouseAndCatExist = false;
    Boolean isListWithHouseAndDogExist = false;
    List&lt;House&gt;[] arrOfHouseList = getArrOfHouseList();
    for(List&lt;House&gt; houseList : arrOfHouseList){

        Set&lt;Boolean&gt; hasCatFlags = houseList.stream().map(House::hasCat).collect(Collectors.toSet());
        if(hasCatFlags.size() &gt; 1){
            return false; //This is mixed case. The list contains both of kind of houses
        }
        
        if (hasCatFlags.contains(true)) {
            isListWithHouseAndCatExist = true;
        } else if (hasCatFlags.contains(false)) {
            isListWithHouseAndDogExist = true;
        }
    }

    return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

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

    Boolean isListWithHouseAndCatExist = false;
    Boolean isListWithHouseAndDogExist = false;
    for(List&lt;House&gt; houseList : getArrOfHouseList()){
        if (houseList.stream().allMatch(House::hasCat)) {
            isListWithHouseAndCatExist = true;
        } else if (houseList.stream().noneMatch(House::hasCat)) {
            isListWithHouseAndDogExist = true;
        } else {
            return false;//mixed case
        } 
    }

    return isListWithHouseAndCatExist &amp;&amp; isListWithHouseAndDogExist;

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

发表评论

匿名网友

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

确定