Better code replacement in nested for loop using Java 8

Question

I am want to avoid this nested for loop and replace it with any better technique in java8. I read about streams in java8 but in this particular code how can I use java8 streams or anything else to make the code better and mainly avoid the nested loop?

List<Country> countryList=new ArrayList<Country>();
List<CountryDTO> countryDtoList=new ArrayList<CountryDTO>();
List<CityDTO> cityDtoList=new ArrayList<CityDTO>();
countryList.forEach(country->{
  CountryDTO countryDto=new CountryDTO();
  countryDto.setCountryId(country.getCountryId());
  countryDto.setCountryName(country.getCountryName());
  countryDto.setCapital(country.getCapital());
  List<City> cityList=new ArrayList<City>();
  cityList=cityRepository.getCitiesForCountry(country.getCountryId());
  cityList.forEach(city->{
    CityDTO cityDto=new CityDTO();
    cityDto.setCityId(city.getCityId());
    cityDto.setCityName(city.getCityName());
    cityDtoList.add(cityDto);
  });
  countryDto.setCities(cityDtoList);
});

Do not use forEach... Go through this docs.oracle.com/javase/tutorial/collections/streams — Tunaki
– Tunaki, Commented Mar 29, 2016 at 9:49
I'd replace forEach by enhanced for loops. There's nothing wrong with nested for loops. — Paul Boddington
– Paul Boddington, Commented Mar 29, 2016 at 9:53
I would consider using constructors. This will make the code much cleaner, and allow you to use mapping easier. — Peter Lawrey
– Peter Lawrey, Commented Mar 29, 2016 at 10:00
@PaulBoddington Ok i will remove the for loop but I dont want to use nested for loop as i heard it is not a good practice. Thanks for the valuable reply. — Akash Raveendran
– Akash Raveendran, Commented Mar 29, 2016 at 10:04

Peter Lawrey · Accepted Answer · 2016-03-29 10:08:16Z

3

I would add a conversion constructor or factory to CountryDTO

List<Country> countryList = ... some data
List<CountryDTO> dtoList = countryList.stream()
                                      .map(CountryDTO::new)
                                      .collect(Collectors.toList());

You would need to add a constructor to CountryDTO

public CountryDTO(Country country) {

alternatively you could use a factory method

public static CountryDTO from(Country country) {

edited Mar 29, 2016 at 10:08

answered Mar 29, 2016 at 10:02

Peter Lawrey

535k83 gold badges770 silver badges1.2k bronze badges

Sign up to request clarification or add additional context in comments.

Comments

assylias · Accepted Answer · 2016-03-29 10:04:09Z

2

You should apply general refactoring techniques and extract the logic in appropriate methods. And it is generally better to use a stream and a series of map calls instead of the forEach method.

List<Country> countryList = ...;
List<CountryDTO> countryDtoList = countryList.stream()
                                             .map(MyClass::countryToDTO)
                                             .collect(toList());

private static CountryDTO countryToDTO(Country country) {
  CountryDTO countryDto=new CountryDTO();
  countryDto.setCountryId(country.getCountryId());
  countryDto.setCountryName(country.getCountryName());
  countryDto.setCapital(country.getCapital());
  List<CityDTO> cityDtoList = cityRepository.getCitiesForCountry(country.getCountryId())
                                            .stream()
                                            .map(MyClass:cityToDTO)
                                            .collect(toList());
  countryDto.setCities(cityDtoList);
  return countryDTO;
}

private static CityDTO cityToDTO(City city) {
  CityDTO cityDto=new CityDTO();
  cityDto.setCityId(city.getCityId());
  cityDto.setCityName(city.getCityName());
  return cityDTO;
}

answered Mar 29, 2016 at 10:04

assylias

330k84 gold badges680 silver badges806 bronze badges

3 Comments

Akash Raveendran Over a year ago

While trying out the code List<CityDTO> cityDtoList = cityRepository.getCitiesForCountry(country.getCountryId()) .stream() .map(MyClass:cityToDTO) .collect(toList()); I am getting an error marking on my IDE saying The type CityDTO does not define CityDTO(City) that is applicable here. I can't guess why such a n error is coming up.

assylias Over a year ago

Are you calling CityDTO::new by any chance? Have you defined such a constructor? If you use the code I've posted, you need to replace MyClass by the class where the cityToDTO static method is located.

Akash Raveendran Over a year ago

Thanks @assylias its fine now...error is no more there.

Collectives™ on Stack Overflow

Better code replacement in nested for loop using Java 8

2 Answers 2

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related