Ocena kodu

0

Napisałem klasę konwertującą i obliczającą na podstawie wejściowego string'a wyrażenie matematyczne podane w wejściowym stringu:

public class rpn
{
    private Dictionary<char, int> priorities;

    public object Arrays { get; private set; }

    public rpn()
    {
        priorities = new Dictionary<char, int>();
        priorities.Add('(', 0);
        priorities.Add('+', 1);
        priorities.Add('-', 1);
        priorities.Add('*', 2);
        priorities.Add('/', 2);
        priorities.Add('^', 3);
    }

    public double calclate(string text)
    {
        List<string> input_list = new List<string>(convert_to_rpn(division(text)));
        double value = 0;
        Stack<double> stack = new Stack<double>();

        double a, b; //zmienne pomocnicze do zapamiętywania wartosci ze stosu

        string asl = String.Empty;
        for (int i = 0; i < input_list.Count; i++)
        {
           // asl = String.Empty;
           // asl = input_list[i];
            //jest liczbą
            if (double.TryParse(input_list[i], out value))
            {
                stack.Push(value);
            }
            else
            { 
                a = stack.Pop(); //zapamiętywanie i ściąganie ze stosu liczb
                b = stack.Pop();
                switch (Convert.ToChar(input_list[i])) 
                {
                    case '+':
                        stack.Push(b + a);
                        break;
                    case '-':
                        stack.Push(b - a);
                        break;
                    case '*':
                        stack.Push(b * a);
                        break;
                    case '/':
                        stack.Push(b / a);
                        break;
                    case '^':
                        stack.Push(Math.Pow(b, a));
                        break;
                }
            }
        }
        return stack.Peek();
    }


    private List<string> convert_to_rpn(String[] input)
    {
        var to_return = new List<string>();
        var stack = new Stack<string>();
        double value;

        for (int i = 0; i < input.Length; i++)
        {
            //jest liczbą
            if (double.TryParse(input[i], out value))
            {
                to_return.Add(input[i]);
            }
            else
            {  
                if (input[i] == "(")
                {
                    stack.Push(input[i]);
                }//jeśli aktualnego element jest prawym nawiasem wszstko do lewego nawiasu wyślij na wyjście i lewy nawias zrzuć
                if (input[i] == ")") 
                {
                    while (stack.Peek() != "(")
                    {
                        to_return.Add(stack.Pop()); 
                    }
                    stack.Pop();
                }
                //aktualnego element jest operatorem
                if (input[i] == "+" || input[i] == "-" || input[i] == "*" || input[i] == "*" || input[i] == "/" || input[i] == "^")
                {
                    if (stack.Count == 0)
                    {
                        stack.Push(input[i]);
                    }//stos nie jest pusty
                    else
                    {
                        if (priorities[Convert.ToChar(input[i])] > priorities[Convert.ToChar(stack.Peek())])
                        {
                            stack.Push(input[i]);
                        }
                        //jeśli priorytet aktualnego elementu jest mniejszy lub równy priorytetowi elementu ze stosu
                        else
                        {
                            while (stack.Count != 0 && priorities[Convert.ToChar(input[i])] <= priorities[Convert.ToChar(stack.Peek())])
                            {
                                to_return.Add(stack.Pop());
                            }
                            stack.Push(input[i]);
                        }
                    }
                }
            }
        }
        while (stack.Count != 0)
        {
            to_return.Add(stack.Pop());
        }
        return to_return;
    }

    //dzielenie wejściowego tekstu na liczby/operatory
    private String[] division(string input_text)
    {
        input_text = input_text.Replace(" ", "");
        input_text = input_text.Replace("+", " + ").Replace("-", " - ").Replace("/", " / ");
        input_text = input_text.Replace("*", " * ").Replace("^", " ^ ");
        input_text = input_text.Replace("(", " ( ").Replace(")", " ) ").Trim();
        String[] to_return = input_text.Split(' ');
        return to_return;
    }
}

Proszę o ocenę i wytknięcie możliwych błędów lub nieprawidłowego podejścia do problemu (program działa poprawnie, chodzi mi o zobaczenie czy mój kod ma cokolwiek związanego z pojęciem dobrego kodu ;) )

0

Nic, ani dobrze ani źle?

4

Skoro Mówisz, że działa to generalnie OK. Brakuje mi jakiegoś opisu jak to uruchamiać, jakiś przykładów, readme - jakie przyjmuje typy danych, jakie są operacje, etc. Co do kodu, to ja bym to podzielił czytelniej na klasy, z jakimiś bardzie opisowymi nazwami: Tokenizer - to Twoje "division"; Eval - Twoje "calclate" i jakieś Parse czy Parser - u Ciebie convert_to_rpn; pozamykać w tych obiektach ich lokalne dane, jak hashmapy, listy, z opisami jak działa algorytm, po co mu ta dana. Ktoś to znajdzie na githubie czy gdzieś i będzie mógł sobie to rozwinąć, zastosować, a tak to będzie dla niego sieczka. I jakaś mozliwość odpalenia tego, np. w pętli while, żeby si,e pobawić potestować. Żeby nie być gołosłownym:

//dzielenie wejściowego tekstu na liczby/operatory
    private String[] division(string input_text)

Trzeba się domyślać, że ta metoda transformuje ciąg wejściowy w listę tokenów.

4

Ciężko ocenić programowanie (w kategoriach OOP) jednej klasy.
Kod działa ? działa :) to językowo jest w miarę ok ;)

Zazwyczaj prze rekrutacjach zleca się proste zadania które wymagają zaimplementowania kilku klas które jakoś wchodzą w interakcje.
Czasem problemu jest "typowy" dla jakiegoś wzorca projektowego aby sprawdzić czy ktoś go użyje. Jak tak -> super, jak nie to trudno, a o wzorce zapytamy w rozmowie.

Mogę wskazać co mi sie średnio podoba:
-nazwa klasy,
-pole typu object o nazwie Arrays :/
-zmienne z _ :)
-kod z ifami albo chociaż warunki zamknąłbym w prywatnych funkcjach, wnętrze warunku:

if (input[i] == "+" || input[i] == "-" || input[i] == "*" || input[i] == "*" || input[i] == "/" || input[i] == "^")

dla mnie jest brzydkie.
Zrobiłbym np prywatna funkcje zwracającą bool i przyjmującą znak.
Fajnie jak funkcje można czytać jak wiersz.

Dużo młodych ludzi (albo zmieniających język z C/C++) myśli że na rekrutacji są "zagadki" jak w C++ :)
Najczęściej jest zadaniem sprawdzające znajomości i rozumienie idee programowania OOP.
Czasem do kodu jakieś testy jednostkowe dopisać (albo wprost zaczynać od nich)...
a po wszystkim....prośba o zmianę funkcjonalności aby sprawdzić czy kod jest elastyczny i jak osoba zareaguje :)

P

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