It looks horrible, but I don't see how can I factorize that ?
I thought of creating small boolean methods but I think it won't change too much, there will always be as many ifs ?
private String getFolderValue(TableRow row) {
String cote = row.getCellValue("B");
String typologie = row.getCellValue("G");
String description = row.getCellValue("Q");
if (cote.startsWith("DE")) {
return "Dessins";
}
if (cote.startsWith("PH")){
return "Photographies";
}
if(cote.startsWith("CA")) {
return "Catalogues";
}
if(cote.startsWith("PU") && typologie.contains("affiche")){
return "Publicité###Affiches";
}
if(cote.startsWith("PU") && typologie.contains("flyer")){
return "Publicité###Flyers";
}
if(cote.startsWith("PU") && description.contains("presse")){
return "Publicité###Presse";
}
if(cote.startsWith("PU") && (description.contains("Facture") || description.contains("devis"))){
return "Documents###Vente";
}
if(typologie.contains("Emballage")){
return "Visual Merchandising###Flyers";
}
if(typologie.contains("PLV")){
return "Visual Merchandising###PLV";
}
if(description.contains("Correspondances")){
return "Documents###Correspondances";
}
return null;
}
CodePudding user response:
Use hashmap to store the data and retrive the data.
CodePudding user response:
In general, a design pattern called
Picture by Vanderjoe -- license: CC BY-SA 4.0
CodePudding user response:
Ok, first of all let's notice that one of your conditions is repeated a lot. Let's try with a nested "if" and let's use StreamAPI.
private String getFolderValue(TableRow row) {
String cote = row.getCellValue("B");
String typologie = row.getCellValue("G");
String description = row.getCellValue("Q");
if (cote.startsWith("DE")) {
return "Dessins";
}
if (cote.startsWith("PH")) {
return "Photographies";
}
if (cote.startsWith("CA")) {
return "Catalogues";
}
if (cote.startsWith("PU")) {
final var topologyContains = Stream.of("flyer", "presse", "affiche").anyMatch(typologie::contains);
if (topologyContains) {
return "Publicité###Affiches";
}
final var descriptionContains = Stream.of("Facture", "devic").anyMatch(description::contains);
if (descriptionContains) {
return "Documents###Vente";
}
}
if (typologie.contains("Emballage")) {
return "Visual Merchandising###Flyers";
}
if (typologie.contains("PLV")) {
return "Visual Merchandising###PLV";
}
if (description.contains("Correspondances")) {
return "Documents###Correspondances";
}
return null;
}
I wouldn't get too much into refactoring here, as this is a simple strategy pattern and in some cases the more "generic" and fancy you go, the worse it is in the end, as it has to be simple to read.
EDIT: Let's divide into functions. You need to test it as I was in a hurry, but you should get the gist:
private String getFolderValue(TableRow row) {
String cote = row.getCellValue("B");
String typologie = row.getCellValue("G");
String description = row.getCellValue("Q");
return Stream.of(
parseFromCote(cote),
parseFromCotePU(cote, typologie, description),
parseFromTypologie(typologie),
parseFromDescription(description)
)
.filter(Objects::nonNull)
.findFirst()
.orElse(null);
}
private String parseFromCote(String cote) {
if (cote.startsWith("DE")) {
return "Dessins";
}
if (cote.startsWith("PH")) {
return "Photographies";
}
if (cote.startsWith("CA")) {
return "Catalogues";
}
return null;
}
private String parseFromCotePU(String cote, String typologie, String description) {
if (cote.startsWith("PU")) {
final var topologyContains = Stream.of("flyer", "presse", "affiche").anyMatch(typologie::contains);
if (topologyContains) {
return "Publicité###Affiches";
}
final var descriptionContains = Stream.of("Facture", "devic").anyMatch(description::contains);
if (descriptionContains) {
return "Documents###Vente";
}
}
return null;
}
private String parseFromTypologie(String typologie) {
if (typologie.contains("Emballage")) {
return "Visual Merchandising###Flyers";
}
if (typologie.contains("PLV")) {
return "Visual Merchandising###PLV";
}
return null;
}
private String parseFromDescription(String description) {
if (description.contains("Correspondances")) {
return "Documents###Correspondances";
}
return null;
}
If you're using Java8 replace final var with proper type. If you prefer to do it lazily you need to pass references to Functional Interfaces there, but I think even "eager" evaluation looks not that bad ;)