Refactoring duplicated code with abstraction and generics

Question

I have written simple command line project with Spring. And from what I think quality of code is pretty poor. I see what I could improve to remove duplicated code, but somehow I have no idea what would be proper way to do it.

So I will copy/paste only code which I think need refactoring. Thos are three separate services which get some data from legacy database(which I cant modify) and dump it to files.

@Service
public class MovieService extends AbstractExportService {
 private final MovieRepository movieRepository;
 private final DecryptDataService decryptDataService;
 public MovieService(
 MovieRepository movieRepository,
 DecryptDataService decryptDataService) {
 this.movieRepository= movieRepository;
 this.decryptDataService = decryptDataService;
 }
 @Transactional
 @Override
 public void extractData(ExtractionParameters extractionParameters) throws IOException {
 Path tempDirectory = Files.createTempDirectory("zip_");
 tempDirectory.toFile().deleteOnExit();
 Set<Movie> movies = movieRepository.findByRidKey(extractionParameters.getWriteNumber());
 for (Movie movie :
 movies) {
 for (ReviewDocuments documents :
 movie.getReviewDocuments()) {
 exportDataToFile(data);
 }
 directoryToZip(tempDirectory.toFile(), movie.getId());
 FileUtils.cleanDirectory(tempDirectory.toFile());
 }
 FileUtils.deleteDirectory(tempDirectory.toFile());
 }
}
@Service
public class BookService extends AbstractExportService {
 private final BookRepository bookRepository;
 private final DecryptDataService decryptDataService;
 public BookService(BookRepository bookRepository, DecryptDataService decryptDataService) {
 this.bookRepository = bookRepository;
 this.decryptDataService = decryptDataService;
 }
 @Transactional
 @Override
 public void extractData(ExtractionParameters extractionParameters) throws IOException {
 Path tempDirectory = Files.createTempDirectory("zip_");
 tempDirectory.toFile().deleteOnExit();
 Set<Book> books = movieRepository.findByRidKey(extractionParameters.getWriteNumber());
 for (Book book :
 books) {
 for (ReviewDocuments documents :
 book.getReviewDocuments()) {
 exportDataToFile(data);
 }
 directoryToZip(tempDirectory.toFile(), book.getId());
 FileUtils.cleanDirectory(tempDirectory.toFile());
 }
 FileUtils.deleteDirectory(tempDirectory.toFile());
 }
}
@Service
public class SongService extends AbstractExportService {
 private final SongRepository songRepository;
 private final DecryptDocumentService decryptDocumentService;
 public SongService(SongRepository songRepository, DecryptDocumentService decryptDocumentService) {
 this.songRepository = songRepository;
 this.decryptDocumentService = decryptDocumentService;
 }
 @Transactional
 @Override
 public void extractData(ExtractionParameters extractionParameters) throws IOException {
 Path tempDirectory = Files.createTempDirectory("zip_");
 tempDirectory.toFile().deleteOnExit();
 Set<Song> songs = songRepository.findByRidKey(extractionParameters.getWriteNumber());
 for (Song song :
 songs) {
 for (ReviewDocuments documents :
 song.getReviewDocuments()) {
 exportDataToFile(documents.getData());
 }
 directoryToZip(tempDirectory.toFile(), song.getId());
 FileUtils.cleanDirectory(tempDirectory.toFile());
 }
 FileUtils.deleteDirectory(tempDirectory.toFile());
 }
}

As you can see, those three classes are pretty similar to each other. What they do is finding List of movies/books/songs depending what user passed as exportParameter, decrypting review documents from database and dumping it to zip files. And the only diffrence in extractData is that we are working on diffrent entities and repository.

DB entities looks like that:

@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
@Entity(name = "MOVIES")
@Table(name = "MOVIES")
public class Movies {
 @Id
 @Column(name = "ID")
 private Long id;
 @Column(name = "MOVIE_KEY")
 private String movieKey;
 @Column(name = "TYPE_ANIMATED")
 private String typeAnim;
 @ManyToMany(fetch = FetchType.LAZY)
 @JoinTable(name = "MOVIES_REVIEWDOCUMENTS",
 joinColumns = @JoinColumn(name = "MOVIE_KEY"),
 inverseJoinColumns = @JoinColumn(name = "DOCUMENT_KEY"))
 private List<ReviewDocuments> reviewDocuments;
}
@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
@Entity(name = "BOOKS")
@Table(name = "BOOKS")
public class Books {
 @Id
 @Column(name = "ID")
 private Long id;
 @Column(name = "BOOK_KEY")
 private String bookKey;
 @Column(name = "TYPE_WRITTEN")
 private String typeWritten;
 @ManyToMany(fetch = FetchType.LAZY)
 @JoinTable(name = "BOOKS_REVIEWDOCUMENTS",
 joinColumns = @JoinColumn(name = "BOOK_KEY"),
 inverseJoinColumns = @JoinColumn(name = "DOCUMENT_KEY"))
 private List<ReviewDocuments> reviewDocuments;
}
@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
@Entity(name = "SONGS")
@Table(name = "SONGS")
public class Songs {
 @Id
 @Column(name = "ID")
 private Long id;
 @Column(name = "SONG_KEY")
 private String songKey;
 @Column(name = "TYPE_SPOKEN")
 private String typeSpoken;
 @ManyToMany(fetch = FetchType.LAZY)
 @JoinTable(name = "SONGS_REVIEWDOCUMENTS",
 joinColumns = @JoinColumn(name = "SONG_KEY"),
 inverseJoinColumns = @JoinColumn(name = "DOCUMENT_KEY"))
 private List<ReviewDocuments> reviewDocuments;
}

I think it would be doable in more generic way to use abstraction in better way. But I need some help.

Answer 1

common Interface classes that share the same behaviour should implement the same interface. That applies to Movie, Book and Song. Their interface is ReviewProvider:

public interface IdProvider {
 long getId();
}
public interface ReviewProvider extends IdProvider{
 List<ReviewDocuments> getReviewDocuments();
}

Refactoring

implementing the Interface for the Entities is quite straight forward, but where there is some work is on each Repository: instead of returning a Set<Movie> / Set<Movie> / Set<Movie> each Repository must provide a Set<ReviewProvider> on the Method findByRidKey. This method must be declared in an shared Interface Repository.

interface Repository{
 Set<ReviewProvider> findByRidKey(long id); //sorry, no code provided here
} 

sorry, there no code provided for there Repositorys, here you are out on your own.

bringing things together

once your entities (movie,book&song) implement ReviewProviderand your Repositorys have been refactored you need only one Transaction: ReviewExportService:

@Service
public class ReviewExportService extends AbstractExportService {
 private final Repository repository;
 private final DecryptDataService decryptDataService;
 public BookService(Repository repository, DecryptDataService decryptDataService) {
 this.repository = repository;
 this.decryptDataService = decryptDataService;
 }
 @Transactional
 @Override
 public void extractData(ExtractionParameters extractionParameters) throws IOException {
 Path tempDirectory = Files.createTempDirectory("zip_");
 tempDirectory.toFile().deleteOnExit();
 Set<ReviewProvider> reviews = repository.findByRidKey(extractionParameters.getWriteNumber());
 for (ReviewProvider review:
 reviews) {
 for (ReviewDocuments documents :
 review.getReviewDocuments()) {
 exportDataToFile(data);
 }
 directoryToZip(tempDirectory.toFile(), review.getId());
 FileUtils.cleanDirectory(tempDirectory.toFile());
 }
 FileUtils.deleteDirectory(tempDirectory.toFile());
 }
}

registering services

when you are done with the previous steps it's easy to register these services:

private final DecryptDocumentService decryptDocumentService;
public void registerServices(){
 //register movies:
 MovieRepository movieRespository = ... 
 registerService(new ReviewExportService(movieRespository, decryptDocumentService);
 //register books:
 BooksRepository booksRespository = ... 
 registerService(new ReviewExportService(booksRespository, decryptDocumentService);
 //register songs:
 SongsRepository songsRespository = ... 
 registerService(new ReviewExportService(songsRespository , decryptDocumentService);
}

• Martin Frank: these questions are a bit out of scope, could you please provide another question, where you provide all code? i will have a look at them!