Va bene se un metodo restituisce una nuova istanza della classe in cui si trova?

1

Ho una classe chiamata MyClass e un metodo chiamato Get che assomiglia a qualcosa del tipo:

public class MyClass{
    string ClassVariable1 {get; set;}
    string ClassVariable2 {get; set;}
    string ClassVariable3 {get; set;}

    public MyClass Get()
    {
           ..set variable1, variable2, and variable3..

                return new MyClass()
                {
                    ClassVariable1 = variable1,
                    ClassVariable2 = variable2,
                    ClassVariable3 = variable3
                };
            }
        }
        return null;
    }
}

C'è qualcosa di sbagliato nel farlo dal punto di vista del design? O qualsiasi altra prospettiva?

Suppongo che per rendere questo più chiaro, pubblicherò il mio codice reale. Sto scrivendo un'app per Facebook e la mia classe si chiama "FacebookAccount". L'idea è che questo contenga cose relative ad un account di Facebook, come nome dello schermo, email dell'utente, recenti aggiornamenti di stato fatti dall'utente, ecc.

class FacebookAccount
{
    public AccessToken AccessToken { get; set; }
    public string ScreeName { get; set; }
    public long FacebookUserId { get; set; }
    public string Email { get; set; }
    public List<StatusUpdate> ScheduledStatusNotPosted { get; set; }
    public List<StatusUpdate> StatusPosted { get; set; }
    private Facebook Facebook { get; set; }

    public List<FacebookAccount> Get(string userId)
    {
        var facebookAccountList = new List<FacebookAccount>();

        using (DataContext dc = new DataContext())
        {
            var facebookAccount = dc.FacebookAccounts.Where(x => x.UserId == userId);

            foreach (var account in facebookAccount)
            {
                var faceBook = new Facebook(account.UserId, account.UserId);
                facebookAccountList.Add(new FacebookAccount()
                {
                    AccessToken = new AccessToken() { Token = account.Token, TokenSecret = string.Empty },
                    Email = account.Email,
                    ScreeName = account.Email,
                    ScheduledStatusNotPosted = faceBook.GetScheduledStatusUpdates(),
                    FacebookUserId = account.UserId
                });
            }
        }

        return facebookAccountList;
    }

    public FacebookAccount Get(string userId, long facebookUserId)
    {
        using (DataContext dc = new DataContext())
        {
            var facebookAccount = dc.FacebookAccounts.FirstOrDefault(x => x.UserId == userId && x.UserId == facebookUserId);
            if (facebookAccount != null)
            {
                Facebook = new Facebook(facebookAccount.UserId, facebookAccount.UserId);

                return new FacebookAccount()
                {
                    AccessToken = new AccessToken() { Token = facebookAccount.Token, TokenSecret = string.Empty},
                    Email = facebookAccount.Email,
                    ScreeName = facebookAccount.Email,
                    ScheduledStatusNotPosted = Facebook.GetScheduledStatusUpdates(),
                    FacebookUserId = facebookAccount.UserId
                };
            }
        }
        return null;
    }
}
    
posta ygetarts 20.08.2015 - 00:02
fonte

3 risposte

7

Non so se è "sbagliato", ma certamente non mi sembra "giusto". Non so perché stai facendo quello che stai facendo. Sembra che tu stia creando istanze, ma non è chiaro se si tratta di nuove istanze o copie.

Se stai creando nuove istanze, il modo usuale per farlo è usare un metodo factory statico. È statico quindi non devi avere un'istanza esistente per creare una nuova istanza.

    static public MyClass Create()
    {
        // set variable1, 2, 3...

        return new MyClass()
        {
            ClassVariable1 = variable1,
            ClassVariable2 = variable2,
            ClassVariable3 = variable3                          
        };
    }

Se questo è un metodo di copia, chiamalo "Copia". Chiamarlo "Get" non ha senso in entrambi i casi.

    
risposta data 20.08.2015 - 00:38
fonte
2

Non è raro avere metodi che restituiscono il tipo della classe - in quale altro modo funzionerebbe una funzione clone?

Tuttavia, non è quello che stai facendo qui. Hai una classe che restituisce un elenco di se stesso. Non credo che questa funzione appartenga alla tua classe di Facebook.

    
risposta data 20.08.2015 - 04:22
fonte
0

I metodi che restituiscono una nuova istanza della loro classe vengono eseguiti per due motivi:

  1. Copia o clonazione dell'oggetto corrente
  2. Rendere l'oggetto immutabile

Il linguaggio C # rende le stringhe immutabili poiché ogni metodo con un tipo di restituzione stringa restituisce una nuova istanza di una stringa. Ciò consente di modificare i dati su un oggetto e di memorizzare il risultato di tale operazione senza modificare l'oggetto originale --- programmazione senza effetti collaterali, se lo si desidera.

Più specificamente alla tua domanda, la classe FacebookAccount viola il Principio di Responsabilità Unica: sta facendo l'accesso ai dati e cose relative ad un account Facebook. Sembra davvero che spostare i metodi di accesso ai dati a FacebookAccountRepository sarebbe più utile qui.

    
risposta data 21.08.2015 - 04:43
fonte

Leggi altre domande sui tag