[JAVA] Ordinare ArrayList di oggetti diversi.

[Dario95]

Salve, so che il titolo può essere fuorviante ma vi spiego subito:
Ho una classe Prodotto, ed una classe figlia ProdottoScontato.
Ho una classe Fattura con un ArrayLIst di tipo Prodotto che mette a disposizione vari metodi e funzioni per aggiungere, eliminare, cercare e per ORDINARE I PRODOTTI IN BASE AL PREZZO.
Ho una classe Test che crea con il 70% di probabilità un Prodotto e con il 30% un ProdottoScontato e li aggiunge ad una Fattura. Usa la funziona ordinaInBase al prezzo della classe Fattura e succede che:
Gli oggetti di Tipo Prodotto vengono ordinati, come volevo, in senso decrescente in base al prezzo, gli oggetti di tipo PrdottoScontato vengono ordinati al contrario e spostati tutti all'inizio della lista. Perchè? Come posso risolvere?
La funzione Ordina di Fattura crea un TreeSet passando un Conparator da me definito e uso addall per aggiungere tutti gli elementi dell'ArrayList di Prodotto, tranne i duplicati.
Dopo vari tentativi ho provato a definire il Comparatore così, so che può sembrare senza senso, e sicuramente lo è ma volevo capire se fosse un problema di cast...

class Comparatore implements Comparator<Object> {


    public int compare(Object o1, Object o2) {


        Prodotto prod1 = null;
        Prodotto prod2 = null;
        if (o1.getClass() == ProdottoScontato.class
                && o2.getClass() == ProdottoScontato.class) {
            prod1 = (ProdottoScontato) o1;
            prod2 = (ProdottoScontato) o2;


           


        } else if (o1.getClass() == Prodotto.class
                && o2.getClass() == ProdottoScontato.class) {
            prod1 = (Prodotto) o1;
            prod2 = (ProdottoScontato) o2;
            

        } else if (o1.getClass() == ProdottoScontato.class
                && o2.getClass() == Prodotto.class) {
            prod1 = (ProdottoScontato) o1;
            prod2 = (Prodotto) o2;


             

        } else if (o1.getClass() == Prodotto.class
                && o2.getClass() == Prodotto.class) {
            prod1 = (Prodotto) o1;
            prod2 = (Prodotto) o2;


                    }
  if (prod1.getPrezzo() > prod2.getPrezzo()) {
                return -1;
            } else if (prod1.getPrezzo() < prod2.getPrezzo()) {
                return 1;
            } else {
                return 0;
            }
     }
}

L'output è il seguente:

26  ProdScontato 1.0
27  ProdScontato 2.0
22  ProdScontato 3.0
10  Prod 8.0
17  Prod 2.0

[schumy2000]

Ciao.
non so se questa uguaglianza riesce (magari si...)
o1.getClass() == ProdottoScontato.class

ma più semplicemente non potresti usare instanceOf?

if (o1 instanceOf ProdottoScontato)....

if (o2 instanceOf Prodotto)....

Secondo me è anche più pulito.

Ciao.

[Dario95]

Ciao.
non so se questa uguaglianza riesce (magari si...)
o1.getClass() == ProdottoScontato.class

ma più semplicemente non potresti usare instanceOf?

if (o1 instanceOf ProdottoScontato)....

if (o2 instanceOf Prodotto)....

Secondo me è anche più pulito.

Ciao.

Si ho provato anche ad usare instanceof ma il risultato non cambiava, quindi ho provato getclass(), e si l'uguaglianza é valida, lo so perché ho utilizzato la modalità debug e quabdo c'era un prodotto e un prodottoScontato entrava nel giusto ramo if..

[andbin]

Gli oggetti di Tipo Prodotto vengono ordinati, come volevo, in senso decrescente in base al prezzo, gli oggetti di tipo PrdottoScontato vengono ordinati al contrario e spostati tutti all'inizio della lista. Perchè? Come posso risolvere?

Dopo vari tentativi ho provato a definire il Comparatore così, so che può sembrare senza senso, e sicuramente lo è ma volevo capire se fosse un problema di cast...

Più che "senza senso" sarebbe meglio dire "contorto" (e anche inutile). Cose del genere generalmente non si fanno.

Tra l'altro guardando il Comparatore c'è una cosa che risulta molto evidente: non fai nulla di specifico con gli oggetti ProdottoScontato. Le due variabili prod1 e prod2 sono di tipo Prodotto (la classe base) che ha il getPrezzo() e solo su questo (che esiste anche in ProdottoScontato) basi la comparazione.

Quindi: perché tutto questo contorcimento solo per comparare sul prezzo, che esiste sia per Prodotto che ProdottoScontato?


P.S. e tra l'altro, visto che usi i generics, almeno parametrizza correttamente: Comparator<Prodotto> e poi il compare(Prodotto p1, Prodotto p2)

[Dario95]

Più che "senza senso" sarebbe meglio dire "contorto" (e anche inutile). Cose del genere generalmente non si fanno.

Tra l'altro guardando il Comparatore c'è una cosa che risulta molto evidente: non fai nulla di specifico con gli oggetti ProdottoScontato. Le due variabili prod1 e prod2 sono di tipo Prodotto (la classe base) che ha il getPrezzo() e solo su questo (che esiste anche in ProdottoScontato) basi la comparazione.

Quindi: perché tutto questo contorcimento solo per comparare sul prezzo, che esiste sia per Prodotto che ProdottoScontato?


P.S. e tra l'altro, visto che usi i generics, almeno parametrizza correttamente: Comparator<Prodotto> e poi il compare(Prodotto p1, Prodotto p2)

E' proprio questo il punto della questione, ed il mio problema, anche se sembra non sia chiaro. Certo il primo Comparatore che ho scritto era:

class Comparatore implements Comparator<Prodotto>{


    public int compare(Prodotto prod1,Prodotto prod2){
            if(prod1.getPrezzo() > prod2.getPrezzo()){
                return -1;
            }else if(prod1.getPrezzo() < prod2.getPrezzo()){
                return 1;
            }else{
                return 0;
            }
     }
}

Facile come bere un bicchier d'acqua.
Ma questo comparatore mi ordina i Prodotti in ordine decrescente per prezzo e in ordine crescente per prezzo i ProdottoScontato, quindi all'inizio della collezione ho tutti ProdottoScontato in ordine crescente e dopo i Prodotto in ordine decrescente.
Pensando fosse un problema di cast ho implementato il comparatore utilazzando quella serie di if e la funzione getclass(), ma il risultato non è cambiato. Come posso ordinare con lo stesso criterio sia i Prodotto che i ProdottoScontato?

Infatti l'output con il comparatore proposto da @andbin è :

//CodiceProdotto Nome Prezzo
18  ProdScontato 4.0
9  Prod 10.0
16  Prod 7.0
14  Prod 6.0
24  Prod 4.0
25  Prod 3.0
27  Prod 2.0

[andbin]

Ma questo comparatore mi ordina i Prodotti in ordine decrescente per prezzo e in ordine crescente per prezzo i ProdottoScontato

No, tecnicamente il comparatore ordina per prezzo decrescente. Stop.

Come posso ordinare con lo stesso criterio sia i Prodotto che i ProdottoScontato?

Ma il comparatore fa già questo .... mostra qualche pezzo di codice (delle due classi Prodotto/ProdottoScontato).

[Dario95]

Ehm si, questo è quello che pensavo anche io, ma...
Come dimostra anche l'output..
Questo è il metodo che utilizza il comparatore:

public Iterator<Prodotto> getandOrderProdotti() {        
TreeSet<Prodotto> listaProdottiOrdinati = new TreeSet<>(new Comparatore());
        listaProdottiOrdinati.addAll(listaProdotti);
        return listaProdottiOrdinati.iterator();
      }
}

listaProdotti è un ArrayList di Prodotto.

Questo è il metodo che stampa l'output:
Il primo ciclo si fa restituire l'iteratore dal metodo mostratovi qui sopra e ne stampa il contenuto.
Il secondo ciclo stampa direttamente l'ArrayList di Prodotti così com'è senza ordinamento e con i duplicati.

for (Iterator<Prodotto> it = fatt.getandOrderProdotti() ; it.hasNext();) {
            System.out.println(it.next());
        }
        
        System.out.println();
        for(Prodotto prod: fatt.listaProdotti){
            System.out.println(prod);
        }
    }

[andbin]

Ehm si, questo è quello che pensavo anche io, ma...
Come dimostra anche l'output..
Questo è il metodo che utilizza il comparatore:

Mostra per cortesia le due classi Prodotto/ProdottoScontato ... a "naso" credo ci sia qualche questione lì.

public Iterator<Prodotto> getandOrderProdotti() {        
TreeSet<Prodotto> listaProdottiOrdinati = new TreeSet<>(new Comparatore());
        listaProdottiOrdinati.addAll(listaProdotti);
        return listaProdottiOrdinati.iterator();
      }
}

Questo tecnicamente è corretto. Ti dò solo 2 dritte:
- innanzitutto se hai pochi elementi non fa molta differenza ma se ne avessi tantissimi (migliaia ecc..), buttarli in un TreeSet solo per ordinarli è molto meno efficiente che clonare una lista ed ordinarla con il sort().
- se non vuoi fornire all'esterno del metodo la informazione che sotto-sotto hai una collezione ordinata, invece che Iterator puoi almeno indicare Iterable (i Set e i List sono Iterable)

public Iterable<Prodotto> getandOrderProdotti() {

In questo modo può essere il "target" del for-each e invece di usare l'iteratore esplicitamente, puoi fare:

for (Prodotto prodotto : fatt.getandOrderProdotti()) { .... }

bello no?

[Dario95]

Il TreeSet l' ho usato perchè mi serviva restituire una collezione senza duplicati. E per quanto riguarda l'iteratore mi viene chiesto esplicitamente di restiuire un iteratore.
Questa è la classe Prodotto:

import java.io.Serializable;
import java.util.Scanner;


public class Prodotto implements Serializable {
    static int codice;
    int codiceProd = 0;
    String nome;
    String descrizione;
    double prezzo;
    Scanner in = new Scanner(System.in);


    public Prodotto(){
        codice++;
        codiceProd = codice;
    }


     public Prodotto(String nome, String descrizione, double prezzo) {
        this();
        this.nome = nome;
        setPrezzo(prezzo);
        this.descrizione = descrizione;
         
    }


    public Prodotto(String nome, double prezzo) {
        this(nome, null, prezzo);
    }


    public void setDescrizione(String descrizione) {
        this.descrizione = descrizione;
    }


    public double getPrezzo() {
        return prezzo;
    }


    public void setPrezzo(double nuovoPrezzo) {
        
        try {
            prezzo = nuovoPrezzo;
            if (nuovoPrezzo < 0) {
                throw new ExceptionPrezzoNegativo();
            }
        } catch (ExceptionPrezzoNegativo e) {
            System.out.println(e.getMessage());
            
            System.out.println("Inserire nuovo prezzo:");
            nuovoPrezzo = in.nextDouble();
            
            setPrezzo(nuovoPrezzo);
            
        }
        
    }


    public String toString() {
        if (descrizione != null) {
            return codiceProd + " " + " " + nome + " " + descrizione + " " + prezzo;
        } else {
            return codiceProd + " " + " " + nome + " " + prezzo;
        }


    }
    
    
    public boolean equals(Object obj) {
        Prodotto prod = (Prodotto) obj;
        if(codiceProd == prod.codiceProd){
            return true;
        }else{
            return false;
        }
    }


}

Questa è la classe ProdottoScontato:

import java.util.Scanner;
public class ProdottoScontato extends Prodotto{
    double sconto;
    Scanner in = new Scanner(System.in);
    public ProdottoScontato(String nome, String descrizione, double prezzo, double sconto) {
        super();
        setSconto(sconto);
        this.nome = nome;
        this.descrizione = descrizione;
        this.prezzo = prezzo;
    }
    public ProdottoScontato(String nome, double prezzo, double sconto) {
        super();
        setSconto(sconto);
        this.nome = nome;
        setPrezzo(prezzo);
    }
    
    
    public double getPrezzo() {
        return sconto * 100 / super.getPrezzo() ;
    }
    
    private void setSconto(double sconto){
        try {
            this.sconto = sconto;
        
        if(this.sconto < 0 || this.sconto > 90){
            throw new ScontoException();
        }
        }catch(ScontoException e){
            System.out.println(e.getMessage());
            System.out.println("Inserire un nuovo sconto valido: ");
            sconto = in.nextDouble();
            setSconto(sconto);
            
            
        }
    }


}

[andbin]

Il TreeSet l' ho usato perchè mi serviva restituire una collezione senza duplicati.

Sì, ma il concetto "senza duplicati" è secondo il criterio fornito dal comparator, quindi sul prezzo. Se hai 3 prodotti con lo stesso prezzo, tiene solo il primo trovato e gli altri li ignora. È davvero quello che vuoi? (giusto per capire io)

E per quanto riguarda l'iteratore mi viene chiesto esplicitamente di restiuire un iteratore.

Ok, se è un requisito ... ovviamente, no prob!

Questa è la classe Prodotto
Questa è la classe ProdottoScontato

Scusa se ti segnalo queste cose ma è doveroso farlo:

- le tue classi fanno "troppe" cose, si occupano anche della interazione (con input!) verso l'utente. Non vanno affatto bene.
E il fatto poi che l'input lo fai nei setPrezzo/setSconto è decisamente brutto e poco pulito, soprattutto per quei try-catch in cui oltretutto invochi ricorsivamente il metodo (noooooo!).

- in setPrezzo di Prodotto prima fai l'assegnamento poi il controllo per negativo. Magari è meglio il contrario! Altrimenti il vincolo non è poi così un vincolo ... sì lanci una eccezione ma lo stato dell'oggetto in quel momento è cambiato.

- abituati e tenere le variabili di istanza tipicamente 'private'.

E ora veniamo al punto importante: getPrezzo in ProdottoScontato è un "override" di quello in Prodotto. Se hai una variabile di tipo Prodotto ma che fa riferimento ad un oggetto ProdottoScontato e invochi prod.getPrezzo(), il getPrezzo invocato è quello dell'oggetto realmente istanziato, ovvero del ProdottoScontato.
In sostanza, il comparatore, comparando i prezzi, concettualmente compara prezzi normali con prezzi "scontati".

E tra l'altro, perché fai:

sconto * 100 / super.getPrezzo()

?? Questo non applica uno sconto ... non ha molto senso ....