Czy to jest kalectwo inżynierii programowania?

0

Hej, jestem wciąż niestety przed zabraniem się porządnie naukę wzorców, choć już niebawem pojadę z koksem, i zastanawiam się czy poniższe rozwiązanie jest czymś praktykowanym?

Na przykład posiadam serwis, w którym posiadam blok kodu:

 else
            {
                await _scrapService.ClickCheckbox(ClickerInput);

                await _scrapService.ClickSome(ClickerInput);
            }

następnie w _scrapService robię coś takiego (żeby przeszedł przez serwis który się zajmuje zbieraniem danych):

public async Task ClickSome(bool clickerInput)
        {
            if (!clickerInput)
            {
                //not applicable yet <- może nigdy tego nie zrobię?
            }
            else
            {
                await _inputService.ClickSometAsync();
            }
        }

        public async Task ClickCheckbox(bool clickerInput)
        {
            if (!clickerInput)
            {
                //not applicable yet <- może nigdy tego nie zrobię?
            }
            else
            {
                await _inputService.ClickCheckboxInputAsync();
            }
        }

żeby w końcu wykonać zadanie w _inputService który się zajmuje symulowaniem kliknięcia:

public async Task ClickSomeInputAsync()
        {
            await MouseLeftButtonClick(236, 892);
        }

        public async Task ClickCheckboxInputAsync()
        {
            await MouseLeftButtonClick(83, 212);
        }

A może po prostu na samym początku powinienem już wołać _inputService? Choć wtedy muszę dodać nową zależność.

Albo inny przykład, gdzie z modelu widoku tak obsługuję komendę przycisku (w tym przypadku nie chciałem zaśmiecać VM):

public async Task OnBuildCommandAsync()
        {
            SearchPhrase = ShowDialog(viewModel => _dialogService.ShowDialog<PhraseView>(this, viewModel));

            await _buttonService.ExecuteBuildButtonAsync(SearchPhrase);
        }

żeby dopiero później w _buttonService wykonać logikę:

public async Task ExecuteBuildButtonAsync(string searchPhrase)
        {
            string callerName = nameof(ExecuteBuildButtonAsync);

            _logger.Info(MessagesInfo[callerName]); //log

            try
            {
                if (!string.IsNullOrEmpty(searchPhrase))
                {
                    await _fileService.SavePhraseAsync(searchPhrase); //file

                    _browseService.UpdateSearchPhrase(searchPhrase);

                    _logger.Info(MessagesResult[callerName] + searchPhrase); //log
                }
                else
                {
                    throw new Exception("Incorrect phrase.");
                }
            }
            catch (Exception e)
            {
                _logger.Error(MessagesError[callerName] + e.Message); //log
            }
        }

Czy wg Was się robi z tego masło maślane i powinienem rozwiązać logikę przycisku w VM?

1

Co do słowa "wzorce" - to nie jest żadna religia, ani za nie-używanie nie palą na stosach.

Wzorzec to stwierdzenie ogólnie znane w kręgu profesjonalistów, powtarzalne. W dodatku wynika w sposób naturalnych z poprzednich doświadczeń branżystów. Np stolarz stolarzowi mówi "połącz to na jaskółczy ogon" i wszytsko jasne.
Byłoby czymś śmiesznym i żałosnym gdyby stolarz dla punktowania wsadził np w stół połowę wzorców jakie zna (pewnie każdą nogę z innego wzorca)

1

Wygląda OK. Zależy co tam jeszcze masz. Jeśli dobrze rozumiem _scrapService to jakiś wrapper, fasada albo adapter na inputService. To ma sens, np jeśli wykorzystujesz kompozycję, żeby dodawać więcej funkcjonalności w każdej następnej warstwie.

Nie wygląda jakby to była sztuka dla sztuki, ale brak mi szerszego kontekstu.

Kilka innych uwag to:
Nie zostawiaj

            //not applicable yet <- może nigdy tego nie zrobię?

bo po co CI to? Chcesz zrobić możesz zostawić '// TODO:' w kodzie (IDE wspiera wyszukiwanie TODOsów. Albo mieć jakiś system ticketów, np trello. Takie puste ścieżki nie prowadzą do niczego dobrego IMO.

Ta zabawa z rzucaniem wyjątku i później łapaniem poniżej też taka sobie. Rozumiem że wyjątek łapiesz i może polecieć też z innego miejsca, ale zamiast rzucać wyjątek w else, możesz zrobić to co w catch masz. Możesz sobie to zamknąć w metodzie prywatnej dla enkapsulacji i pokazania że ma być to samo.

1

Nie odp na temat w komentarzach. Teraz muszę Ci odp w poście i dziwnie to wygląda :D Możesz zrobić np tak:


            try
            {
... nie wazne
                }
                else
                {
                    _logger.Error(MessagesError[callerName] + "Incorrect phrase."); //log
                }
            }
            catch (Exception e)
            {
                _logger.Error(MessagesError[callerName] + e.Message); //log
            }
        }

albo tak:


            try
            {
... nie wazne
                }
                else
                {
                    LogError("Incorrect phrase."); 
                }
            }
            catch (Exception e)
            {
                 LogError(e.Message); 
            }
        }

Gdzie LogError to prywatna metoda.

1 użytkowników online, w tym zalogowanych: 0, gości: 1