Code Verkürzen?

Redga

Mitglied
Hallo Leute,

Ich habe eine Frage zu folgendem Code.
Java:
    public boolean synchronize(ManualSynchronizationForm form) {

        List<String> modules = Arrays.asList(form.getCheckedEntities());
        Collection<String> organizations = Arrays.asList(form.getCheckedOrganizations());
        String fromDate = form.getFromDate();
        String toDate = form.getToDate();
        String customerIds = form.getCustomerId();
        String branchIds = form.getBranchId();
        String salesfloorIds = form.getSalesfloorId();
        String customerNo = null;
        String branchNo = null;
        String salesfloorNo = null;

        boolean result = true;
        
         if (!StringUtils.isEmpty(customerIds) && !StringUtils.isEmpty(branchIds) && !StringUtils.isEmpty(salesfloorIds)) {
            
            List<Customer> filteredCustomer = null;
            CacheDS cache = null;
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = cache.getCustomers();
                filteredCustomer = Customers.getCustomersForId(filteredCustomer, custId.intValue());

                StringBuilder custNo = new StringBuilder();
                for (Customer customer : filteredCustomer) {
                    custNo.append(customer.getNo());
                    custNo.append(",");
                }
                customerNo = custNo.substring(0, custNo.lastIndexOf(","));
            }

            List<Branch> filteredBranchs = null;
            String[] branIds = branchIds.split(",");
            for (final String branchId : branIds) {
                Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                filteredBranchs = cache.getBranches();
                filteredBranchs = Branches.getBranchesForId(filteredBranchs, branId.intValue());

                StringBuilder branNo = new StringBuilder();
                for (Branch branch : filteredBranchs) {
                    branNo.append(branch.getNo());
                    branNo.append(",");
                }
                customerNo = branNo.substring(0, branNo.lastIndexOf(","));
            }

            List<Salesfloor> filteredSalesfloors = null;
            String[] sfloorIds = salesfloorIds.split(",");
            for (final String salesfloorId : sfloorIds) {
                Integer sfloorId = StringUtils.isNumeric(salesfloorId) ? Integer.valueOf(salesfloorId) : null;

                filteredSalesfloors = cache.getSalesfloors();
                filteredSalesfloors = Salesfloors.getSalesfloorsForId(filteredSalesfloors, sfloorId.intValue());

                StringBuilder sfloorNo = new StringBuilder();
                for (Salesfloor salesfloor : filteredSalesfloors) {
                    sfloorNo.append(salesfloor.getNo());
                    sfloorNo.append(",");
                }
                customerNo = sfloorNo.substring(0, sfloorNo.lastIndexOf(","));
            }
        }
        
        if (!StringUtils.isEmpty(customerIds) && StringUtils.isEmpty(branchIds) && StringUtils.isEmpty(salesfloorIds)) {
                                                                                                                            
            final Set<Salesfloor> setSF = new HashSet<Salesfloor>();
            final Set<Branch> setB = new HashSet<Branch>();
            String[] custIds = customerIds.split(",");
            CacheDS cache = null;
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            List<Salesfloor> filteredSalesfloors = null;
            List<Branch> filteredBranches = null;
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                if (filteredSalesfloors == null) {
                    filteredSalesfloors = new ArrayList<>();
                }

                if (filteredBranches == null) {
                    filteredBranches = new ArrayList<>();
                }

                if (custId != null) {
                    filteredSalesfloors = Salesfloors.getSalesfloorsForCustomerId(cache.getSalesfloors(),
                            custId.intValue());

                    filteredBranches = Branches.getBranchesForCustomerId(cache.getBranches(), custId.intValue());
                }

                if (filteredSalesfloors != null && !filteredSalesfloors.isEmpty()) {
                    setSF.addAll(filteredSalesfloors);
                }

                if (filteredBranches != null && !filteredBranches.isEmpty()) {
                    setB.addAll(filteredBranches);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Customer " + custId + " has no salesfloors");
                    return false;
                }
            }

            StringBuilder branNo = new StringBuilder();
            for (Branch branch : filteredBranches) {

                branNo.append(branch.getNo());
                branNo.append(",");
            }
            branchNo = branNo.substring(0, branNo.lastIndexOf(","));

            StringBuilder sfloorNo = new StringBuilder();
            for (Salesfloor salesfloor : filteredSalesfloors) {

                sfloorNo.append(salesfloor.getNo());
                sfloorNo.append(",");
            }
            salesfloorNo = sfloorNo.substring(0, sfloorNo.lastIndexOf(","));

            List<Customer> filteredCustomer = null;
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = cache.getCustomers();
                filteredCustomer = Customers.getCustomersForId(filteredCustomer, custId.intValue());

                StringBuilder custNo = new StringBuilder();
                for (Customer customer : filteredCustomer) {
                    custNo.append(customer.getNo());
                    custNo.append(",");
                }
                customerNo = custNo.substring(0, custNo.lastIndexOf(","));
            }

        } else if (!StringUtils.isEmpty(customerIds) && !StringUtils.isEmpty(branchIds)
                && StringUtils.isEmpty(salesfloorIds)) {

            final Set<Salesfloor> setSF = new HashSet<Salesfloor>();
            String[] branIds = branchIds.split(",");
            CacheDS cache = null;
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }
            // for Sf
            List<Salesfloor> filteredSalesfloors = null;
            for (final String branchId : branIds) {
                Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                if (filteredSalesfloors == null) {
                    filteredSalesfloors = new ArrayList<>();
                }

                if (branIds != null) {
                    filteredSalesfloors = Salesfloors.getSalesfloorsForBranchId(cache.getSalesfloors(),
                            branId.intValue());
                }

                if (filteredSalesfloors != null && !filteredSalesfloors.isEmpty()) {
                    setSF.addAll(filteredSalesfloors);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Branch " + branId + " has no salesfloors");
                    return false;
                }
            }

            List<Customer> filteredCustomer = null;
            String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = cache.getCustomers();
                filteredCustomer = Customers.getCustomersForId(filteredCustomer, custId.intValue());

                StringBuilder custNo = new StringBuilder();
                for (Customer customer : filteredCustomer) {
                    custNo.append(customer.getNo());
                    custNo.append(",");
                }
                customerNo = custNo.substring(0, custNo.lastIndexOf(","));

                List<Branch> filteredBranchs = null;
                for (final String branchId : branIds) {
                    Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                    filteredBranchs = cache.getBranches();
                    filteredBranchs = Branches.getBranchesForId(filteredBranchs, branId.intValue());

                    StringBuilder branNo = new StringBuilder();
                    for (Branch branch : filteredBranchs) {
                        branNo.append(branch.getNo());
                        branNo.append(",");
                    }
                    customerNo = branNo.substring(0, branNo.lastIndexOf(","));
                }

                StringBuilder sfloorNo = new StringBuilder();
                for (Salesfloor salesfloor : filteredSalesfloors) {

                    sfloorNo.append(salesfloor.getNo());
                    sfloorNo.append(",");
                }
                salesfloorNo = sfloorNo.substring(0, sfloorNo.lastIndexOf(","));
            }

        }

Wie man sieht ist er recht groß geworden weil er ständig die selbe Logik verwendet.
Wie kann ich ihn am besten reduzieren bzw. kann ich eine Methode bauen um gewisse Vorgänge zu wiederholen?

Danke für eure Mühen!
 

MoxxiManagarm

Top Contributor
Was mir in dem code am meisten ins Auge sticht sind die StringBuilders.

Ersetze sowas:
Java:
StringBuilder custNo = new StringBuilder();
                for (Customer customer : filteredCustomer) {
                    custNo.append(customer.getNo());
                    custNo.append(",");
                }
                customerNo = custNo.substring(0, custNo.lastIndexOf(","));

Mit sowas:
Java:
customerNo = filteredCustomer.stream().map(customer -> customer.getNo()).collect(Collectors.joining(","));
Du verwendest hier übrigens wiederholt customerNo auch bei Branches und Floors


Du brauchst die Zwischenspeicher beim Split nicht.

Java:
 for (final String customerId : customerIds.split(","))
 

Redga

Mitglied
Hey Leute
Ich habe an sowas hier gedacht?
Jetzt müssen nur irgendwie die filteredCustomer und die customerNo übertragen werde. Könnt ihr mir da einen Tipp geben?

Java:
    private void buildStringbuilderForCustomer(){
            StringBuilder custNo = new StringBuilder();
            for (Customer customer : filteredCustomer) {
                custNo.append(customer.getNo());
                custNo.append(",");
            }
            customerNo = custNo.substring(0, custNo.lastIndexOf(","));}
        }
    
    private void buildStringbuilderForBranch(){
        StringBuilder branNo = new StringBuilder();
        for (Branch branch : filteredBranches) {

            branNo.append(branch.getNo());
            branNo.append(",");
        }
        branchNo = branNo.substring(0, branNo.lastIndexOf(","));}
    }
    
    private void buildStringbuilderForSalesfloor(){
        StringBuilder sfloorNo = new StringBuilder();
        for (Salesfloor salesfloor : filteredSalesfloors) {

            sfloorNo.append(salesfloor.getNo());
            sfloorNo.append(",");
        }
        salesfloorNo = sfloorNo.substring(0, sfloorNo.lastIndexOf(","));}

Grüße!
 

MoxxiManagarm

Top Contributor
Könnt ihr mir da einen Tipp geben?
Da sowohl Customer, Branch als auch Salesfloor die Methode getNo() besitzen, würde ich hoffen, dass sie ein gemeinsames Interface implementieren oder von der selben Klasse erben. Nennen wir wir diese schlicht NoItem.

Java:
private String joinNoItemList(List<NoItem> items) {
        StringBuilder sb= new StringBuilder();
        for (NoItem item : items) {
            sb.append(item.getNo());
            sb.append(",");
        }
        return sb.substring(0, sb.lastIndexOf(","));}
}

Wobei ich persönlich weiterhin den Vorschlag mit dem Stream siehe oben bevorzugen würde...

Java:
private String joinNoItemList(List<NoItem> items) {
  return items.stream().map(item -> item.getNo()).collect(Collectors.joining(","));
}

Einsetzen tust du es dann so:

Java:
customerNo = joinNoItem(filteredCustomer);
branchNo = joinNoItem(filteredBranch);
salesfloorNo = joinNoItem(filteredSalesfloor);

Ein Hinweis noch, dir sollte aber bewusst sein, dass das...

Java:
String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = cache.getCustomers();
                filteredCustomer = Customers.getCustomersForId(filteredCustomer, custId.intValue());

                customerNo = joinNoItem(filteredCustomer);
            }
... nur diese letzte Iteration über custIds berücksichtigt, da du dir mit jeder Iteration den Inhalt von customerNo überschreibst.

Außerdem macht die doppelte Zuweisung auf filteredCustomer auch keinen Sinn, mach aus den beiden Zeilen das:
Java:
filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), custId.intValue());

Und noch was...
Java:
Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;
filteredCustomer = Customers.getCustomersForId( cache.getCustomers(), custId.intValue());

Keine gute Idee, da eine NullPointerException fliegen wird sobald customerId einmal nicht numerisch ist, denn custId ist dann null und mit custId.intValue() würdest du versuchen diese Methode auf null auszuführen.

Besser:
Java:
if(StringUtils.isNumeric(customerId)) {
  int custId = Integer.parseInt(customerId);
  // do something with custId
} else {
  // handle error case
}
 
Zuletzt bearbeitet:

mrBrown

Super-Moderator
Mitarbeiter
Du musst auch keine Liste implementieren, du musst nur die Liste an die aufgerufene Funktion übergeben.
Der methodenkopf muss dazu etwa so aussehen: String buildStringbuilderForCustomer(List<Customer> filteredCustomer)
 

Redga

Mitglied
Hallo Leute.
Ich habe es hinbekommen!
Danke dafür und für eure Geduld mit mir :)

Mein Code sieht jetzt so aus:

Java:
private String buildStringbuilderForCommaSeparated(List<? extends Entity> filteredDataList, String customerNo) {
        StringBuilder custNo = new StringBuilder();
        for (Entity entity : filteredDataList) {
            custNo.append(entity.getNo());
            custNo.append(",");
        }
        customerNo = custNo.substring(0, custNo.lastIndexOf(","));
        return customerNo;
    }

    public boolean synchronize(ManualSynchronizationForm form) {

        List<String> modules = Arrays.asList(form.getCheckedEntities());
        List<Customer> filteredCustomer = null;
        List<Salesfloor> filteredSalesfloors = null;
        List<Branch> filteredBranches = null;
        Collection<String> organizations = Arrays.asList(form.getCheckedOrganizations());
        CacheDS cache = null;
        String fromDate = form.getFromDate();
        String toDate = form.getToDate();
        String customerIds = form.getCustomerId();
        String branchIds = form.getBranchId();
        String salesfloorIds = form.getSalesfloorId();
        String customerNo = null;
        String branchNo = null;
        String salesfloorNo = null;

        boolean result = true;

        try {
            cache = CacheService.getInstance().getCacheDS();
        } catch (CacheServiceException e1) {
            log.fatal(e1);
        }

        // TODO wenn alle vorhanden
        if (!StringUtils.isEmpty(customerIds) && !StringUtils.isEmpty(branchIds)
                && !StringUtils.isEmpty(salesfloorIds)) {

            String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), custId.intValue());
            }

            String[] branIds = branchIds.split(",");
            for (final String branchId : branIds) {
                Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                filteredBranches = Branches.getBranchesForId(cache.getBranches(), branId.intValue());
            }

            String[] sfloorIds = salesfloorIds.split(",");
            for (final String salesfloorId : sfloorIds) {
                Integer sfloorId = StringUtils.isNumeric(salesfloorId) ? Integer.valueOf(salesfloorId) : null;

                filteredSalesfloors = Salesfloors.getSalesfloorsForId(cache.getSalesfloors(), sfloorId.intValue());
            }

        } else
        // TODO wenn cust aber kein bran sf
        if (!StringUtils.isEmpty(customerIds) && StringUtils.isEmpty(branchIds) && StringUtils.isEmpty(salesfloorIds)) {

            final Set<Salesfloor> setSF = new HashSet<Salesfloor>();
            final Set<Branch> setB = new HashSet<Branch>();
            String[] custIds = customerIds.split(",");
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            if (filteredSalesfloors == null) {
                filteredSalesfloors = new ArrayList<>();
            }

            if (filteredBranches == null) {
                filteredBranches = new ArrayList<>();
            }

            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                if (custId != null) {
                    filteredSalesfloors = Salesfloors.getSalesfloorsForCustomerId(cache.getSalesfloors(),
                            custId.intValue());

                    filteredBranches = Branches.getBranchesForCustomerId(cache.getBranches(), custId.intValue());
                }

                if (filteredSalesfloors != null && !filteredSalesfloors.isEmpty()) {
                    setSF.addAll(filteredSalesfloors);
                }

                if (filteredBranches != null && !filteredBranches.isEmpty()) {
                    setB.addAll(filteredBranches);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Customer " + custId + " has no branches or salesfloors");
                    return false;
                }
            }
//TODO wenn cust / bran aber kein sf
        } else if (!StringUtils.isEmpty(customerIds) && !StringUtils.isEmpty(branchIds)
                && StringUtils.isEmpty(salesfloorIds)) {

            final Set<Salesfloor> setSF = new HashSet<Salesfloor>();
            String[] branIds = branchIds.split(",");

            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            if (filteredSalesfloors == null) {
                filteredSalesfloors = new ArrayList<>();
            }

            for (final String branchId : branIds) {
                Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                if (branIds != null) {
                    filteredSalesfloors = Salesfloors.getSalesfloorsForBranchId(cache.getSalesfloors(),
                            branId.intValue());
                }

                if (filteredSalesfloors != null && !filteredSalesfloors.isEmpty()) {
                    setSF.addAll(filteredSalesfloors);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Branch " + branId + " has no salesfloors");
                    return false;
                }
            }

            String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), custId.intValue());

                for (final String branchId : branIds) {
                    Integer branId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                    filteredBranches = Branches.getBranchesForId(cache.getBranches(), branId.intValue());
                }
            }

        }
//TODO Wenn Branch aber kein Cust oder Salesfloor
        else if (StringUtils.isEmpty(customerIds) && !StringUtils.isEmpty(branchIds)
                && StringUtils.isEmpty(salesfloorIds)) {

            final Set<Customer> setC = new HashSet<Customer>();
            final Set<Salesfloor> setSF = new HashSet<Salesfloor>();
            String[] branIds = branchIds.split(",");
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            for (final String branchId : branIds) {
                Integer banId = StringUtils.isNumeric(branchId) ? Integer.valueOf(branchId) : null;

                if (filteredSalesfloors == null) {
                    filteredSalesfloors = new ArrayList<>();
                }

                if (filteredCustomer == null) {
                    filteredCustomer = new ArrayList<>();
                }

                Branch branch = cache.getBranch(banId);

                if (banId != null) {
                    filteredSalesfloors = Salesfloors.getSalesfloorsForBranchId(cache.getSalesfloors(),
                            banId.intValue());

                    filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), branch.getCustomerId());
                }

                if (filteredSalesfloors != null && !filteredSalesfloors.isEmpty()) {
                    setSF.addAll(filteredSalesfloors);
                }

                if (filteredCustomer != null && !filteredCustomer.isEmpty()) {
                    setC.addAll(filteredCustomer);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Customer " + banId + " has no salesfloors");
                    return false;
                }
            }
        }

//TODO Wenn sFloors aber kein Cust und kein Branch
        else if (StringUtils.isEmpty(customerIds) && StringUtils.isEmpty(branchIds)
                && !StringUtils.isEmpty(salesfloorIds)) {

            final Set<Customer> setC = new HashSet<Customer>();
            final Set<Branch> setB = new HashSet<Branch>();
            String[] sfloorIds = salesfloorIds.split(",");
            try {
                cache = CacheService.getInstance().getCacheDS();
            } catch (CacheServiceException e1) {
                log.fatal(e1);
            }

            for (final String salesfloorId : sfloorIds) {
                Integer sfloorId = StringUtils.isNumeric(salesfloorId) ? Integer.valueOf(salesfloorId) : null;

                if (filteredCustomer == null) {
                    filteredCustomer = new ArrayList<>();
                }

                if (filteredBranches == null) {
                    filteredBranches = new ArrayList<>();
                }

                Salesfloor salesfloor = cache.getSalesfloor(sfloorId);
                Branch branch = cache.getBranch(salesfloor.getBranchId());

                if (salesfloorId != null) {
                    filteredBranches = Branches.getBranchesForId(cache.getBranches(), salesfloor.getBranchId());

                    filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), branch.getCustomerId());
                }

                if (filteredCustomer != null && !filteredCustomer.isEmpty()) {
                    setC.addAll(filteredCustomer);
                }

                if (filteredBranches != null && !filteredBranches.isEmpty()) {
                    setB.addAll(filteredBranches);
                }

                if (filteredSalesfloors == null) {
                    log.fatal("Customer " + sfloorId + " has no salesfloors");
                    return false;
                }
            }
        }
        
        customerNo = buildStringbuilderForCommaSeparated(filteredCustomer, customerNo);
        branchNo = buildStringbuilderForCommaSeparated(filteredBranches, branchNo);
        salesfloorNo = buildStringbuilderForCommaSeparated(filteredSalesfloors, salesfloorNo);



Meine Letzte Frage wäre eigt nurnoch wie ich die For-Schleifen "fixen" kann.
Hat da Jemand eine Idee?
 

Redga

Mitglied
Java:
String[] custIds = customerIds.split(",");
            for (final String customerId : custIds) {
                Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;

                filteredCustomer = cache.getCustomers();
                filteredCustomer = Customers.getCustomersForId(filteredCustomer, custId.intValue());

                customerNo = joinNoItem(filteredCustomer);
            }

Wie kann man diese Schleife umbauen das nicht immer der selbe Wert gezogen wird?


Gruß
 

MoxxiManagarm

Top Contributor
Mein Code sieht jetzt so aus:
Das wird immernoch eine NullPointerException werfen sobald der Wert nicht numerisch ist
Java:
Integer custId = StringUtils.isNumeric(customerId) ? Integer.valueOf(customerId) : null;
filteredCustomer = Customers.getCustomersForId(cache.getCustomers(), custId.intValue());

Sonar würde hier z.B. anmerken, dass der Zwischenspeicher unnötig ist. Das hatte ich ebenfalls bereits erwähnt
Java:
// statt
String[] custIds = customerIds.split(",");
for (final String customerId : custIds) {

// das
for (final String customerId : customerIds.split(",")) {

Der 2. Parameter in deiner Methode buildStringbuilderForCommaSeparated ist nicht notwendig
 

mrBrown

Super-Moderator
Mitarbeiter
Nein, das Problem ist, dass customerNo mit jeder Iteration überschrieben wird und daher nur die letzte Iterion über custIds berücksichtigt wird. Redga, custoemrNo könnte z.B. eine Liste von Strings sein, die du jeweils ergänzt und am Ende erneut zusammen führst.
Das meinte ich mit nicht korrekt :p
Ob ne Schleife da richtiger ist, mag ich nicht zu beurteilen, vielleicht stecken da noch ganz andere Probleme drin...
 

Redga

Mitglied
Danke ich versuche mal einiges Umzusetzen :)
Es ist wirklich eine ältere Java Version die ich verwenden darf :´D

Vielen Dank für die Hilfe. Ich versuche mich erstmal daran die For Schleife zu korrigieren
 

MoxxiManagarm

Top Contributor
Es ist wirklich eine ältere Java Version die ich verwenden darf :´D
Auf welcher Version arbeitest du, unabhängig davon auf welcher Version der Code ursprünglich erstellt wurde?

Für Java8+ habe ich besten Gewissens den Code einmal im Texteditor umgebaut (alles ohne Gewähr)
Java:
private String joinEntitySet(Set<? extends Entity> entities) {
    return entities.stream().map(entity -> entity.getNo()).collect(Collectors.joining(","));
}

private <E extends Entity> List<E> filterCacheList(List<E> entities, ToIntFunction<E> keyExtractor, int id) {
    return entities.stream().filter(entity -> keyExtractor.applyAsInt(entity) == id).collect(Collectors.toList());
}

public boolean synchronize(ManualSynchronizationForm form) {

    CacheDS cache = null;

    List<String> modules = Arrays.asList(form.getCheckedEntities());
    Collection<String> organizations = Arrays.asList(form.getCheckedOrganizations());        
    
    String fromDate = form.getFromDate();
    String toDate = form.getToDate();
    String customerIds = form.getCustomerId();
    String branchIds = form.getBranchId();
    String salesfloorIds = form.getSalesfloorId();
    
    Set<Customer> filteredCustomer = new HashSet<>();
    Set<Salesfloor> filteredSalesfloors = new HashSet<>();
    Set<Branch> filteredBranches = new HashSet<>();
    
    String customerNo = null;
    String branchNo = null;
    String salesfloorNo = null;

    boolean result = true;
    
    // Step 0: cache
    try {
        cache = CacheService.getInstance().getCacheDS();
    } catch (CacheServiceException e1) {
        log.fatal("cache couldn't be instanced");
        return false;
    }
    
    // Step 1: customerNo
    // get customers from cache
    if(!StringUtils.isEmpty(customerIds)) {
        for (final String customerId : customerIds.split(",")) {
            if(StringUtils.isNumeric(customerId)) {
                filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getCustomerId, Integer.parseInt(customerId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            } 
        }
    }
    // get customers from branches & salesfloors
    else { 
        // get customers by branches
        if(!StringUtils.isEmpty(branchIds)) { 
            for (final String branchId : branchIds.split(",")) {
                if(StringUtils.isNumeric(branchId)) {
                    filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getBranchId, Integer.parseInt(branchId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
        
        // get customer by salesfloors
        if(!StringUtils.isEmpty(salesfloorIds)) { 
            for (final String salesfloorId : salesfloorIds.split(",")) {
                if(StringUtils.isNumeric(salesfloorId)) {
                    filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getSalesfloorId, Integer.parseInt(salesfloorId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
    }
    if (filteredCustomer.isEmpty()) {
        log.fatal("no customer found");
        return false;
    }
    
    // Step 2: branchNo
    // get branches from cache
    if(!StringUtils.isEmpty(branchIds)) { 
        for (final String branchId : branchIds.split(",")) {
            if(StringUtils.isNumeric(branchId)) {
                filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch::getBranchId, Integer.parseInt(branchId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            } 
        }
    }
    // get branches from customers and salesfloors
    else {
        // get branches by customers
        if(!StringUtils.isEmpty(customerIds)) { 
            for (final String customerId : customerIds.split(",")) {
                if(StringUtils.isNumeric(customerId)) {
                    filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch::getCustomerId, Integer.parseInt(customerId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
        
        // get branches by salesfloors
        if(!StringUtils.isEmpty(salesfloorIds)) { 
            for (final String salesfloorId : salesfloorIds.split(",")) {
                if(StringUtils.isNumeric(salesfloorId)) {
                    filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch:getSalesfloorId, Integer.parseInt(salesfloorId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
    }
    if (filteredBranches.isEmpty()) {
        log.fatal("no branch found");
        return false;
    }
    
    // Step 3: salesfloorNo
    // get salesfloors from cache
    if(!StringUtils.isEmpty(salesfloorIds)) { 
        for (final String salesfloorId : salesfloorIds.split(",")) {
            if(StringUtils.isNumeric(salesfloorId)) {
                filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getSalesfloorId, Integer.parseInt(salesfloorId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            } 
        }
    }
    // get salesfloors from customers & branches
    else {
        // get salesfloor by customers
        if(!StringUtils.isEmpty(customerIds)) { 
            for (final String customerId : customerIds.split(",")) {
                if(StringUtils.isNumeric(customerId)) {
                    filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getCustomerId, Integer.parseInt(customerId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
        
        // get salesfloors by branches
        if(!StringUtils.isEmpty(branchIds)) { 
            for (final String branchId : branchIds.split(",")) {
                if(StringUtils.isNumeric(branchId)) {
                    filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getBranchId, Integer.parseInt(branchId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                } 
            }
        }
    }
    if (filteredSalesfloors.isEmpty()) {
        log.fatal("no salesfloor found");
        return false;
    }
    
    // TODO: Achtung, die Variablen sind lokal und
    // stehen in dieser Version außerhalb dieser Methode nicht zur Verfügung
    customerNo = joinEntitySet(filteredCustomer);
    branchNo = joinEntitySet(filteredBranches);
    salesfloorNo = joinEntitySet(filteredSalesfloors);
    
    return true;
}

Vermutlich kann man das noch weiter minimieren, aber ich hatte dann keine Lust mehr ^^
 

Redga

Mitglied
Auf welcher Version arbeitest du, unabhängig davon auf welcher Version der Code ursprünglich erstellt wurde?

Für Java8+ habe ich besten Gewissens den Code einmal im Texteditor umgebaut (alles ohne Gewähr)
Java:
private String joinEntitySet(Set<? extends Entity> entities) {
    return entities.stream().map(entity -> entity.getNo()).collect(Collectors.joining(","));
}

private <E extends Entity> List<E> filterCacheList(List<E> entities, ToIntFunction<E> keyExtractor, int id) {
    return entities.stream().filter(entity -> keyExtractor.applyAsInt(entity) == id).collect(Collectors.toList());
}

public boolean synchronize(ManualSynchronizationForm form) {

    CacheDS cache = null;

    List<String> modules = Arrays.asList(form.getCheckedEntities());
    Collection<String> organizations = Arrays.asList(form.getCheckedOrganizations());       
   
    String fromDate = form.getFromDate();
    String toDate = form.getToDate();
    String customerIds = form.getCustomerId();
    String branchIds = form.getBranchId();
    String salesfloorIds = form.getSalesfloorId();
   
    Set<Customer> filteredCustomer = new HashSet<>();
    Set<Salesfloor> filteredSalesfloors = new HashSet<>();
    Set<Branch> filteredBranches = new HashSet<>();
   
    String customerNo = null;
    String branchNo = null;
    String salesfloorNo = null;

    boolean result = true;
   
    // Step 0: cache
    try {
        cache = CacheService.getInstance().getCacheDS();
    } catch (CacheServiceException e1) {
        log.fatal("cache couldn't be instanced");
        return false;
    }
   
    // Step 1: customerNo
    // get customers from cache
    if(!StringUtils.isEmpty(customerIds)) {
        for (final String customerId : customerIds.split(",")) {
            if(StringUtils.isNumeric(customerId)) {
                filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getCustomerId, Integer.parseInt(customerId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            }
        }
    }
    // get customers from branches & salesfloors
    else {
        // get customers by branches
        if(!StringUtils.isEmpty(branchIds)) {
            for (final String branchId : branchIds.split(",")) {
                if(StringUtils.isNumeric(branchId)) {
                    filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getBranchId, Integer.parseInt(branchId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
       
        // get customer by salesfloors
        if(!StringUtils.isEmpty(salesfloorIds)) {
            for (final String salesfloorId : salesfloorIds.split(",")) {
                if(StringUtils.isNumeric(salesfloorId)) {
                    filteredCustomer.addAll(filterCacheList(cache.getCustomers(), Customer::getSalesfloorId, Integer.parseInt(salesfloorId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
    }
    if (filteredCustomer.isEmpty()) {
        log.fatal("no customer found");
        return false;
    }
   
    // Step 2: branchNo
    // get branches from cache
    if(!StringUtils.isEmpty(branchIds)) {
        for (final String branchId : branchIds.split(",")) {
            if(StringUtils.isNumeric(branchId)) {
                filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch::getBranchId, Integer.parseInt(branchId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            }
        }
    }
    // get branches from customers and salesfloors
    else {
        // get branches by customers
        if(!StringUtils.isEmpty(customerIds)) {
            for (final String customerId : customerIds.split(",")) {
                if(StringUtils.isNumeric(customerId)) {
                    filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch::getCustomerId, Integer.parseInt(customerId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
       
        // get branches by salesfloors
        if(!StringUtils.isEmpty(salesfloorIds)) {
            for (final String salesfloorId : salesfloorIds.split(",")) {
                if(StringUtils.isNumeric(salesfloorId)) {
                    filteredBranches.addAll(filterCacheList(cache.getBranches(), Branch:getSalesfloorId, Integer.parseInt(salesfloorId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
    }
    if (filteredBranches.isEmpty()) {
        log.fatal("no branch found");
        return false;
    }
   
    // Step 3: salesfloorNo
    // get salesfloors from cache
    if(!StringUtils.isEmpty(salesfloorIds)) {
        for (final String salesfloorId : salesfloorIds.split(",")) {
            if(StringUtils.isNumeric(salesfloorId)) {
                filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getSalesfloorId, Integer.parseInt(salesfloorId)));
            } else {
                log.fatal("form contained malformed id(s)");
                return false;
            }
        }
    }
    // get salesfloors from customers & branches
    else {
        // get salesfloor by customers
        if(!StringUtils.isEmpty(customerIds)) {
            for (final String customerId : customerIds.split(",")) {
                if(StringUtils.isNumeric(customerId)) {
                    filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getCustomerId, Integer.parseInt(customerId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
       
        // get salesfloors by branches
        if(!StringUtils.isEmpty(branchIds)) {
            for (final String branchId : branchIds.split(",")) {
                if(StringUtils.isNumeric(branchId)) {
                    filteredSalesfloors.addAll(filterCacheList(cache.getSalesfloors(), Salesfloor::getBranchId, Integer.parseInt(branchId)));
                } else {
                    log.fatal("form contained malformed id(s)");
                    return false;
                }
            }
        }
    }
    if (filteredSalesfloors.isEmpty()) {
        log.fatal("no salesfloor found");
        return false;
    }
   
    // TODO: Achtung, die Variablen sind lokal und
    // stehen in dieser Version außerhalb dieser Methode nicht zur Verfügung
    customerNo = joinEntitySet(filteredCustomer);
    branchNo = joinEntitySet(filteredBranches);
    salesfloorNo = joinEntitySet(filteredSalesfloors);
   
    return true;
}

Vermutlich kann man das noch weiter minimieren, aber ich hatte dann keine Lust mehr ^^


Wahnsinn danke für die Mühen!
Ich habe alles umgesetzt bekommen und somit fertig mit meinen Aufgaben für das Praktikum. Danke für die Unterstützungen von euch!
 

MoxxiManagarm

Top Contributor
Ein Gedanke wie man mein Beispiel noch weiter reduzieren könnte...
Java:
filteredCustomer = new CacheFilter(cache.getCustomers())
  .by(new Argument(Customer::getCustomerId, customerIds))
  .otherwiseBy(new Argument(Customer::getBranchId, branchIds))
  .otherwiseBy(new Argument(Customer::getSalesfloorId, salesfloorIds))
  //ODER: .otherwiseBy(new Argument(Customer::getBranchId, branchIds), new Argument(Customer::getSalesfloorId, salesfloorIds))
  .execute();
// angelehnte Signatur für Branches
// angelehnte Signatur für Salesfloors

Das ist aber so erstmal kein funktionierender Code, den CacheFilter müsste man bauen, er würde aber aus den 100+ Zeilen sich wiederholenden Code von Step1-3 deutlich weniger machen. Durch das Method-Chaining wäre er außerdem noch etwas leslicher.


PS.: Arguments sollten den Gedanken nur verdeutlichen, vermutlich wären hier Predicate besser.
 
Ähnliche Java Themen
  Titel Forum Antworten Datum
Antoras Code Verkürzen Java Basics - Anfänger-Themen 8
M Code verkürzen Java Basics - Anfänger-Themen 7
A code verkürzen Java Basics - Anfänger-Themen 4
T Kann mir jemand wörtlich erklären, was in dem Code genau passiert? Java Basics - Anfänger-Themen 1
Ü Dead Code im Programm? Java Basics - Anfänger-Themen 13
I QR code in Java selber generieren Java Basics - Anfänger-Themen 5
terashy VS Code Project run error Java Basics - Anfänger-Themen 10
JaZuDemNo Code Erklärung Java Basics - Anfänger-Themen 3
M Connect-4-Code analysieren Java Basics - Anfänger-Themen 2
N BMI Rechner Was haltet ihr von dem Code habt ihr Verbesserungsvorschläge weil design teschnisch ist das nicht das geilste würde das gerne überarbeiten Java Basics - Anfänger-Themen 12
W In alten Code zurück- und dort wieder zurechtfinden? Java Basics - Anfänger-Themen 17
T code so schreiben das er von sich selber anpasst (code soll die anzahl aller bustaben bestimmen) Java Basics - Anfänger-Themen 16
J Frage zu einem "Taschenrechner" code Java Basics - Anfänger-Themen 9
T Fehlercode bei code der das Alter ausrechnet Java Basics - Anfänger-Themen 2
T Text einlesen code was kommt dahin? Java Basics - Anfänger-Themen 1
jhfjeh Strukturgramm in code Java Basics - Anfänger-Themen 11
D Tipps zum Code Java Basics - Anfänger-Themen 24
W Java-Code mit Array Java Basics - Anfänger-Themen 14
W Java-Code Java Basics - Anfänger-Themen 2
W Java code- TicTac toe Java Basics - Anfänger-Themen 51
W Java-code Java Basics - Anfänger-Themen 8
W Java-code Java Basics - Anfänger-Themen 9
W Java-Code erklären Java Basics - Anfänger-Themen 6
ohneInformatik; For Schleife. Was macht dieser Code?? Java Basics - Anfänger-Themen 5
Say Fehlenden Code finden in einer while-Schleife? Java Basics - Anfänger-Themen 11
Say 2-DIM Array Code lesen und verstehen Java Basics - Anfänger-Themen 5
Say Stelle in Code herausfinden, wie geht man vor? Java Basics - Anfänger-Themen 12
Say do-While Code Ausführung Java Basics - Anfänger-Themen 3
W Rückfrage zur Programmgestaltung (clean code) Java Basics - Anfänger-Themen 12
M intelliJ auf neuem PC, plötzlich kein Code Java Basics - Anfänger-Themen 3
Pinhg Sound in Greenfoot Code einbinden Java Basics - Anfänger-Themen 2
C Java boolean Code läuft nicht Java Basics - Anfänger-Themen 5
I Code für Bezahlsystem (auch bei Offline Aktivität) Java Basics - Anfänger-Themen 7
J Größter gemeinsamer Teiler: mein Code Java Basics - Anfänger-Themen 6
B Den Dateipfad einer Java Datei durch Code in Selbiger finden? Java Basics - Anfänger-Themen 10
A Wie könnte man diesen Code kürzer machen ? Java Basics - Anfänger-Themen 7
J Frage zu meinem Code (OOP) Java Basics - Anfänger-Themen 4
Alen123 Warum funktioniert mein Code nicht? Java Basics - Anfänger-Themen 64
Max246Sch Frage zu Währungsrechner Code Java Basics - Anfänger-Themen 2
S Hilfe bei Umänderung von Java Code Java Basics - Anfänger-Themen 16
I Code wird nicht ausgeführt Java Basics - Anfänger-Themen 2
K Wie kann man diesen Code schnell und effizient interpretieren (Man hat nur 4 Minuten) Java Basics - Anfänger-Themen 3
R ISBN-10-Code überprüfen Java Basics - Anfänger-Themen 7
I Bitte um Hilfe zu unterstehenden Code Java Basics - Anfänger-Themen 6
I Interface von einer EJB Klasse, um Code zu reduzieren Java Basics - Anfänger-Themen 1
I HTML Code säubern Java Basics - Anfänger-Themen 4
B Brauche Hilfe zu einem Code Java Basics - Anfänger-Themen 5
Temsky34 Problem mit dem Code Java Basics - Anfänger-Themen 17
N Fehler im Code (Aufgabe für Anfänger) Java Basics - Anfänger-Themen 11
N Java-Code abwärtskompatibel machen Java Basics - Anfänger-Themen 4
J Erste Schritte Was mache ich in meinem Code falsch. Java Basics - Anfänger-Themen 3
Ameise04 Variablen Inhalt einer Variable im Code verwenden? Java Basics - Anfänger-Themen 9
S Compiler-Fehler Nicht adressierbarer Code ( Non-addressable code ) Java Basics - Anfänger-Themen 5
Aemulit Java Schaltjahr berechnen Code Java Basics - Anfänger-Themen 7
A Code Problem Java Basics - Anfänger-Themen 6
C Fehler im Code Java Basics - Anfänger-Themen 10
A Zu einem bestimmten Ort im Code springen Java Basics - Anfänger-Themen 11
L Ist der Code richtig Java Basics - Anfänger-Themen 3
josfe1234 code vereinfachen Java Basics - Anfänger-Themen 15
nonickatall Ausführbarkeit von Code testen bzw. Remote Debugging Java Basics - Anfänger-Themen 4
F Frage betreff Programm mit dem man C++-Code in JAVA-Code übersetzen lassen kann Java Basics - Anfänger-Themen 2
S Fehler bei Code mit SubStrings für mich nicht auffindbar. Java Basics - Anfänger-Themen 4
G Programm Code Java Basics - Anfänger-Themen 5
C Code zusammenfassen Java Basics - Anfänger-Themen 5
I Erklärung zum Java Code Java Basics - Anfänger-Themen 2
T Programmablaufsplaninterpretation in Code umformen Java Basics - Anfänger-Themen 1
dieter000 Kurze Frage kann mir ejmand kurz diesen Code erklären, bzw wie man die zeilen erklärt und so Java Basics - Anfänger-Themen 1
AlexVo String zu Java Anweisung getString("*** java code ***") Java Basics - Anfänger-Themen 19
M ISBN-Code Java Basics - Anfänger-Themen 26
B Zeitgleiches Arbeiten am Code mit mehreren Personen? Java Basics - Anfänger-Themen 7
S Wie kann ich bei diesem Code erreichen, das als Ergebnis hier 15 herauskommt? Java Basics - Anfänger-Themen 23
N Kann man den Code vereinfachen? Java Basics - Anfänger-Themen 25
marcooooo Code erklären Java Basics - Anfänger-Themen 28
marcooooo Code erklären Java Basics - Anfänger-Themen 4
S Advent of Code Day4 Java Basics - Anfänger-Themen 4
B Nach eingefügtem Code erkennt Compiler keine Instanzvar und meldet SyntaxError Java Basics - Anfänger-Themen 2
Gaudimagspam Caesars Code entziffern in Java Java Basics - Anfänger-Themen 8
Lukasbsc Wie kann ich meinen Code optimieren? Java Basics - Anfänger-Themen 4
NeoLexx equals()-Methode Verständnis Frage anhand Code Beispiel Java Basics - Anfänger-Themen 22
I Input/Output Code wird doppelt ausgeführt Java Basics - Anfänger-Themen 3
T Main startet nicht bei vorgegebenen Code Java Basics - Anfänger-Themen 41
B Frage zum Code verständnis im Resultat Java Basics - Anfänger-Themen 10
J Fehler im Code, aber ich weiß nicht wieso! Java Basics - Anfänger-Themen 6
S Mehrere Probleme im Code Java Basics - Anfänger-Themen 7
M Code nur für Cracks? Crack the Passwort Übung Java Basics - Anfänger-Themen 7
parrot Code entferneJedeZweiteZiffer Java Basics - Anfänger-Themen 6
G Code kürzen Java Basics - Anfänger-Themen 5
Bluedaishi Source Code Signieren Java Basics - Anfänger-Themen 22
L Best Practice Code Refactoring für Methoden mit fast gleicher Aufbau Java Basics - Anfänger-Themen 6
L Best Practice Code refactern Java Basics - Anfänger-Themen 30
G code kürzen Java Basics - Anfänger-Themen 16
A Code umschreiben Java Basics - Anfänger-Themen 6
Torsten.E JavaFX mit Visual Studio Code verwenden Java Basics - Anfänger-Themen 1
C Beispiel-Code mit Pair wird nicht compiliert. Java Basics - Anfänger-Themen 8
X Reverse algorithm engineering (Java code) Java Basics - Anfänger-Themen 6
T Bufferedwriter code Nullpointerexception Java Basics - Anfänger-Themen 4
V Switch Methode macht Code kaputt Java Basics - Anfänger-Themen 18
R Was muss ich an meinem Code ändern? Java Basics - Anfänger-Themen 2
S Bewertet meinen Code - Part 1 Java Basics - Anfänger-Themen 8
M Java Code Verständnis Java Basics - Anfänger-Themen 4

Ähnliche Java Themen

Neue Themen


Oben