英文:
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<House>[] arrOfHouseList = getArrOfHouseList();
for(List<House> houseList : arrOfHouseList){
Boolean isHousesWithCat = true;
Boolean isHousesWithDog = true;
for(House house: houseList){
if(house.hasCat()){
isHousesWithDog = false;
}else{
isHousesWithCat = false;
}
}
if(!isHousesWithDog && ! 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<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; //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 && 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<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; //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 && 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<House> 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 && isListWithHouseAndDogExist;
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库,让每个人都能够通过互相帮助和分享经验来进步。
评论