Single Responsibility - questa classe sta facendo troppo?

3

Sto riprogettando uno dei miei programmi che esegue determinate azioni sui processi di interesse (noti come "Processi monitorati" nel mio programma).

Alcune azioni I sempre che devono essere eseguite su tali processi sono:

  • Apri l'handle del processo
  • Ottieni il nome del processo
  • Chiudi l'handle del processo

E alcune altre azioni che potrebbero dover fare su alcune o tutte sono:

  • Sospendi processo
  • Processo di ripresa
  • Imposta priorità del processo
  • Imposta affinità di processo

In precedenza avevo semplicemente una classe "gestore" che apriva un handle per il processo, ne controllava il nome ed eseguiva le azioni richieste. Tutto ciò avviene tramite p / invoke, non uso affatto la classe Process di .NET.

Tuttavia sto cercando di ridisegnarlo da una prospettiva SOLID, quindi ho creato un oggetto "MonitoredProcess" e ho aggiunto un numero di metodi per eseguire le azioni.

Qui è nel suo stato attuale:

public class MonitoredProcess : IMonitoredProcess
{
    private readonly IMonitoredProcessConfig config;
    private readonly int processId;
    private IntPtr handle = IntPtr.Zero;
    private uint suspendResumeResult;
    private string processName;

    public IMonitoredProcessConfig Config { get { return config; } }
    public int ProcessId { get { return processId; } }
    public uint SuspendResumeResult { get { return suspendResumeResult; } }
    public string ProcessName
    {
        get
        {
            if (processName == null) PopulateProcessName();

            return processName;
        }
    }

    public MonitoredProcess(IMonitoredProcessConfig config, int processId)
    {
        if (config == null) throw new ArgumentNullException("config");

        this.config = config;
        this.processId = processId;
    }


    public bool OpenProcess()
    {
        var alreadyOpen = handle != IntPtr.Zero;
        if (alreadyOpen) return true;

        handle = Kernel32.OpenProcess(config.RequiredRights, false, (uint)processId);
        return handle != IntPtr.Zero;
    }

    public bool CloseProcess()
    {
        var closed = Kernel32.CloseHandle(handle);
        if (closed) handle = IntPtr.Zero;
        return closed;
    }

    public uint Resume()
    {
        suspendResumeResult = Ntdll.NtResumeProcess(handle);
        return suspendResumeResult;
    }

    public uint Suspend()
    {
        suspendResumeResult = Ntdll.NtSuspendProcess(handle);
        return suspendResumeResult;
    }

    private void PopulateProcessName()
    {
        var buffer = new StringBuilder((int)Kernel32.GeneralConstants.MAX_PATH);
        var success = 0 != Psapi.GetModuleFileNameEx(
            handle,
            IntPtr.Zero,
            buffer,
            (uint)buffer.Capacity
            );

        processName = success ? Path.GetFileName(buffer.ToString()) : string.Empty;
    }

}

Non ho ancora aggiunto metodi per impostare la priorità e l'affinità del processo. L'interfaccia che implementa non è ancora del tutto definita, diventerà ciò che alla fine la classe esporrà.

Domanda:

Questa classe sta facendo troppo? Un "processo" dovrebbe sapere come sospendere / riprendere se stesso e impostare la propria priorità e l'affinità della CPU? O dovrei avere altre classi che prendono un IMonitoredProcess ed eseguono queste azioni su di loro (che è come l'ho fatto prima)?

Nel modo in cui l'ho fatto prima, le classi che eseguivano le azioni le implementavano tramite p / invoke, quindi "sapevano" che potevano lanciare e catturare Win32Exception s per ottenere il messaggio di errore. Se questa classe MonitoredProcess implementa tutto tramite p / invoke internamente, la classe che chiama tecnicamente questi metodi non dovrebbe sapere come sono implementati, quindi non ha motivo di lanciare o catturare Win32Exception s.

E non vorrei semplicemente buttarli da questa classe (anche avvolti in un'eccezione specifica del dominio) perché in metà dei luoghi chiamo i metodi non mi interessa il risultato, ma avrei comunque per provare / catturarlo ogni volta - e crea oggetti inutilmente.

Che di nuovo mi fa chiedere se questa classe dovrebbe davvero fare tutte queste azioni, apparentemente su se stessa, ma usa funzioni esterne per eseguirle tutte.

    
posta 404 15.01.2016 - 19:15
fonte

3 risposte

4

Chiamerei la classe ProcessController piuttosto che MonitoredProcess. L'istanza di classe non è un processo, incapsula un handle di processo. E non solo controlla, controlla.

Potresti passare l'id del processo al costruttore.

In questo modo la terminologia sarà corretta, sarebbe chiaro che la tua classe controlla un processo per te. Qual è una responsabilità. Quindi stai bene.

    
risposta data 15.01.2016 - 21:42
fonte
1

Questo dipende in gran parte dal contesto di un "MonitoredProcess" nell'applicazione nel suo complesso.

Guarda il nome che hai usato: Monitorato Processo.

Ciò implica che il processo sia monitorato da qualcosa . Può essere una classe, un metodo o qualcos'altro nell'applicazione che sta monitorando i processi e decide se vuole o meno eseguire queste azioni.

In base a ciò è necessario determinare se il monitor stesso è più interessato all'esecuzione di azioni sui processi o al tracciamento dei risultati delle azioni sui processi. Nella prima situazione, sarebbe il monitor che vuole essere in grado di eseguire l'operazione Open() , Close() , Resume() e Suspend() su un processo, non il MonitoredProcess che vuole eseguire queste azioni su se stesso. In quest'ultimo, MonitoredProcess vorrebbe contenere queste azioni e il monitor vorrebbe semplicemente richiamare queste operazioni sul processo stesso e gestire i risultati.

    
risposta data 15.01.2016 - 20:26
fonte
1

Il modo più utile che ho trovato per comprendere il Principio di Responsabilità Unica è interpretare la "responsabilità" come "motivo per cui il codice potrebbe cambiare". Questo perché l'obiettivo principale di SRP è quello di raggruppare il codice che cambierà insieme e (per quanto possibile) mantenere il codice che cambierà per diversi motivi separati da esso. Facendo ciò, renderemo più semplice apportare tali modifiche se mai richiesto.

Nel tuo caso, l'unica ragione per cui dovresti cambiare il codice è se il codice di sistema sottostante che stai chiamando tramite p / invoca le modifiche (o perché una nuova versione di Windows cambia API in futuro, o se decidere di trasferire il progetto su un sistema operativo diverso).

Da questo punto di vista, il tuo codice va bene, anche se può valere la pena (se puoi mai prevedere la possibilità di una modifica del SO) raggruppando questo codice insieme a qualsiasi altro codice dipendente dal sistema, in una sottodirectory o in un progetto di biblioteca.

L'unico cambiamento che farei è quello di incapsulare i tuoi valori di ritorno in qualcosa di più significativo di un uint generico - così com'è, i tuoi clienti devono capire l'implementazione di Win32 sottostante, che è una violazione della Legge di Demetra. Crea un tipo di reso specifico che incapsula il valore restituito da Windows.

    
risposta data 16.01.2016 - 09:41
fonte

Leggi altre domande sui tag