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.