Hw1 bwt by AnNyiiik · Pull Request #2 · AnNyiiik/HWSecondTerm

AnNyiiik · 2023-02-22T21:12:07Z

No description provided.

…beticFrequencySequence & SortCharacters in BWT class

viktoriia-fomina · 2023-02-23T21:44:38Z

HW1_BWT/HW1_BWT/HW1_BWT.csproj

+
+    <PropertyGroup>
+        <OutputType>Exe</OutputType>
+        <TargetFramework>net5.0</TargetFramework>


Перейдите, пожалуйста, на .NET 7 :)

viktoriia-fomina · 2023-02-23T21:45:53Z

HW1_BWT/HW1_BWT/BWT.cs

@@ -0,0 +1,166 @@
+using System;
+using System.Linq;


Неиспользуемый using. Надо убрать

viktoriia-fomina · 2023-02-23T21:48:11Z

HW1_BWT/HW1_BWT/BWT.cs

+
+namespace HW1_BWT
+{
+    public class BWT


У вас все методы static и поле тоже. Класс тоже стоит сделать static

viktoriia-fomina · 2023-02-23T22:00:28Z

HW1_BWT/HW1_BWT/BWT.cs

+        public static string Encode(string sequence)
+        {
+            if (sequence.Length <= 1)
+            {


А еще значение строки может быть null. Тогда ваша программа упадет с исключением NullPointerException.
Хорошо бы случаях, когда sequence == null || sequence.Length <= 0 возвращать, например, null - значение, которое будет индикатором того, что входные данные были некорректные

Тут я не соглашусь, отсутствие значения должно быть явным, а null возвращать не надо никогда (и стандартная библиотека .NET никогда его не возвращает). Иначе уже вызывающий словит NullReferenceException внезапно для себя. И, кстати, в .NET 7 хорошо работает nullability-анализ — попытка передать или вернуть что-то, что может быть null, вызовет недовольство компилятора.

viktoriia-fomina · 2023-02-23T22:19:05Z

HW1_BWT/HW1_BWT/BWT.cs

+                }
+            }
+
+            return String.Join("", encodedSequence);


return new string(encodedSequence) вероятно будет несколько понятнее выглядеть

viktoriia-fomina · 2023-02-23T22:37:28Z

HW1_BWT/HW1_BWT/BWT.cs

+            return String.Join("", encodedSequence);
+        }
+
+        public static string Decode(string sequence)


Для обратного преобразования Барроуза-Уилера нам нужна строка и позиция (это написало в условии задачи, кстати).
Ваш класс - это API для работы с преобразованием Барроуза-Уилера. Вы можете захотеть когда-то где-то его переиспользовать и сами же пострадаете от своего кода.
Что плохо: вы рассчитываете, что пользователь сначала вызовет Decode и только после этого вызова вызовет Encode (организация записи и чтения из поля POSITION). То есть вы рассчитываете на определенную последовательность вызовов методов. Но вам ее никто гарантировать не может.
Если вызвать Decode перед Encode, то ваша программа будет работать некорректно. Поправьте, пожалуйста :)

viktoriia-fomina · 2023-02-23T22:45:12Z

HW1_BWT/HW1_BWT/BWT.cs

+                currentPosition = vector[currentPosition];
+            }
+
+            return String.Join("", result);


Тут тоже return new string(result) вероятно будет несколько понятнее выглядеть

viktoriia-fomina · 2023-02-23T22:49:34Z

HW1_BWT/HW1_BWT/Program.cs

+                Console.WriteLine("Tests have been failed");
+                return;
+            }
+            string code = BWT.Encode("banana");


Из условия: "На вход подаётся строка, на выходе должна получиться строка, преобразованная Барроузом-Уилером, и позиция конца строки в результате преобразования."

Во-первых, надо дать пользователю возможность ввести строку, чтобы выполнить преобразование, а не хардкодить ее.
Во-вторых, нужно вернуть не только строку - результат преобразования, но и позицию

viktoriia-fomina · 2023-02-23T22:52:19Z

HW1_BWT/HW1_BWT/Program.cs

+            }
+            string code = BWT.Encode("banana");
+            Console.WriteLine(code);
+            string encoded = BWT.Decode(code);


Из условия: "Реализовать также и обратное преобразование, принимающее преобразованную строку и позицию, и возвращающую исходную строку."

Надо дать пользователю возможность ввести строку и позицию

viktoriia-fomina · 2023-02-23T22:55:04Z

HW1_BWT/HW1_BWT/Program.cs

+            string code = BWT.Encode("banana");
+            Console.WriteLine(code);
+            string encoded = BWT.Decode(code);
+            Console.WriteLine(encoded);


Сейчас ваша программа вообще не общается с пользователем. Ему, например, непонятно, что в консоль выводится.

Пользовательский ввод обязательно должен сопровождаться подсказками, а вывод в консоль, какой-то информацией, поясняющей, что собственно вывелось

… to encode, fixed test

added input output and fixed test encode

added input output

viktoriia-fomina · 2023-03-02T18:01:04Z

HW1_BWT/HW1_BWT/BWT.cs

@@ -0,0 +1,162 @@
+using System;
+
+namespace HW1_BWT


Используйте file-scoped namespaces вместо namespaces с фигурными скобочками.
И еще пользуйтесь top-level statements

viktoriia-fomina · 2023-03-02T18:10:06Z

HW1_BWT/HW1_BWT/BWT.cs

+            {
+                return Tuple.Create(sequence, 0);
+            }
+            int[] permutationsPositions = new int[sequence.Length];


Используйте var - аналог auto в C++.
Это

int[] permutationsPositions = new int[sequence.Length];

можно записать так:

var permutationsPositions = new int[sequence.Length];

, где тип левой части будет на самом деле известен компилятору по правой части выражения

viktoriia-fomina · 2023-03-02T18:10:36Z

HW1_BWT/HW1_BWT/BWT.cs

+                permutationsPositions[i] = i;
+            }
+            SortPermutations(permutationsPositions, sequence);
+            char[] encodedSequence = new char[sequence.Length];


Снова можно использовать var

viktoriia-fomina · 2023-03-02T18:12:00Z

HW1_BWT/HW1_BWT/BWT.cs

+            }
+            SortPermutations(permutationsPositions, sequence);
+            char[] encodedSequence = new char[sequence.Length];
+            int position = 0;


Ненужное присваивание.
В .NET у типов есть значения по умолчанию, у int это 0. Достаточно просто int position;

viktoriia-fomina · 2023-03-02T18:13:58Z

HW1_BWT/HW1_BWT/Test.cs

+    {
+        private static bool TestEncode()
+        {
+            string[] testCases = new[] { "abracadabra", "banana", "abacaba", ""};


Даже тут можно var использовать

viktoriia-fomina · 2023-03-02T18:21:03Z

HW1_BWT/HW1_BWT/Test.cs

+        private static bool TestEncode()
+        {
+            string[] testCases = new[] { "abracadabra", "banana", "abacaba", ""};
+            string[] correctAnswers = new[] { "rdarcaaaabb", "nnbaaa", "bcabaaa", ""};


Пробел потеряли при инициализации массива и лучше с var. То есть:

var correctAnswers = new[] { "rdarcaaaabb", "nnbaaa", "bcabaaa", "" };

viktoriia-fomina · 2023-03-02T18:21:48Z

HW1_BWT/HW1_BWT/Test.cs

+
+        private static bool TestDecode()
+        {
+            string[] testCases = new[] { "rdarcaaaabb", "bcabaaa"};


Снова пробел потерян при инициализации массива и можно var использовать

viktoriia-fomina · 2023-03-02T18:27:14Z

HW1_BWT/HW1_BWT/Test.cs

+            {
+                Tuple<string, int> answer = BWT.Encode(testCases[i]);
+                if (String.Compare(answer.Item1, correctAnswers[i]) != 0 
+                    || correctPositions[i] != answer.Item2)


Если что-то не умещается в строку, то отступ не 4, а 8 пробелов.

if (String.Compare(answer.Item1, correctAnswers[i]) != 0 || correctPositions[i] != answer.Item2)

viktoriia-fomina · 2023-03-02T18:43:07Z

HW1_BWT/HW1_BWT/BWT.cs

+            int[] firstPositionsOfSortedCharacters = new int[cardinality];
+            char[] alphabet = new char[cardinality];
+            GetAlphabeticFrequencySequence(sequence, firstPositionsOfSortedCharacters, alphabet);


Метод Get..., который ничего не возвращает. Такое название путает.

Вы создаете 2 пустых массива, чтобы потом с ними какие-то манипуляции в методе GetAlphabeticFrequencySequence произвести. Создавайте тогда массивы в этом методе. А метод GetAlphabeticFrequencySequence может вернуть объект класса, в который вы захотите обернуть возвращаемое значение (2 ваших массива, который вам дальше нужны). В крайнем случае можно их в кортеже вернуть, это будет несколько менее понятно, но пока так можно при условии, что добавите к методу комментарии о смысле возвращаемого значения

viktoriia-fomina · 2023-03-02T18:46:31Z

HW1_BWT/HW1_BWT/Program.cs

+                return;
+            }
+
+            if (option == 1)


Здесь switch... case будет несколько лучше смотреться

viktoriia-fomina · 2023-03-11T14:25:53Z

HW1_BWT/HW1_BWT/BWT.cs

@@ -0,0 +1,161 @@
+using System;
+
+namespace HW1_BWT;


Нужна пустая строка после namespace

viktoriia-fomina · 2023-03-11T14:27:15Z

HW1_BWT/HW1_BWT/BWT.cs

+using System;
+
+namespace HW1_BWT;
+public static class BWT {


Скобочку по стайлгайду для C# надо на следующую строку перенести

viktoriia-fomina · 2023-03-11T14:28:05Z

HW1_BWT/HW1_BWT/Test.cs

@@ -0,0 +1,43 @@
+using System;
+
+namespace HW1_BWT;


Опять-таки нужна пустая строка после namespace

viktoriia-fomina · 2023-03-11T14:28:38Z

HW1_BWT/HW1_BWT/Test.cs

+using System;
+
+namespace HW1_BWT;
+public class Test {


И опять нужно скобочку на следующую строку перенести.

Уже гораздо лучше, но стоит внимательнее проверять на корректность то, что ввел пользователь и то, что передается в качестве аргументов в публичные методы. Поправьте, пожалуйста, эти замечания и можно будет оценку поднять на балл

Проверять на корректность аргумент в самом методе?

Проверять на корректность аргумент в самом методе?

Мы с Юрием Викторовичем договорились, что будем требовать от вас для public методов проверку на всё. Для internal методов проверку на null можно не делать, потому что nullability-анализ не позволит вам просто так взять и передать в метод null

viktoriia-fomina · 2023-03-11T14:29:13Z

HW1_BWT/HW1_BWT/Program.cs

+    if (!Test.TestBWT())
+    {
+        Console.WriteLine("Tests have been failed");
+        return;
+    }
+    Console.WriteLine("Please, enter the option (1 to encode and 2 to decode):");
+    var isCorrect = Int32.TryParse(Console.ReadLine(), out int option);
+    if (!isCorrect)
+    {
+        Console.WriteLine("Not a number");
+        return;
+    }
+
+    switch (option)
+    {
+        case 1:
+            Console.WriteLine("Please, enter a string to encode:");
+            var sequence = Console.ReadLine();
+            if (sequence == null)
+            {
+                Console.WriteLine("Empty string");
+                return;
+            }
+            var answer = BWT.Encode(sequence);
+            Console.WriteLine("Encoded string : {0} \nPosition : {1}", answer.Item1, answer.Item2);
+            break;
+
+        case 2:
+            Console.WriteLine("Please, enter a string to decode:");
+            sequence = Console.ReadLine();
+            if (sequence == null)
+            {
+                Console.WriteLine("Empty string");
+                return;
+            }
+            Console.WriteLine("Please, enter a position:");
+            isCorrect = Int32.TryParse(Console.ReadLine(), out int position);
+            if (!isCorrect)
+            {
+                Console.WriteLine("Not a number");
+                return;
+            } 
+            if (position >= sequence.Length || position < 0)
+            {
+                Console.WriteLine("Position is incorrect");
+                return;
+            }
+            var decodedString = BWT.Decode(sequence, position);
+            Console.WriteLine("Decoded string : {0} ", decodedString);
+            break;
+
+        default:
+            Console.WriteLine("Option is incorrect");
+            break;
+    }


Сместите все, пожалуйста, на 4 пробела влево

viktoriia-fomina · 2023-03-11T14:36:58Z

HW1_BWT/HW1_BWT/BWT.cs

+        return Tuple.Create(new string(encodedSequence), position);
+    }
+
+    public static string Decode(string sequence, int position)


Тут тоже стоило проверить, что sequence не null, а position находится в корректном диапазоне значений

viktoriia-fomina · 2023-03-11T14:38:20Z

HW1_BWT/HW1_BWT/BWT.cs

+        }
+    }
+
+    static bool ComparePermutations(int first, int second, string sequenceZero)


Давайте договоримся явно указывать модификаторы видимости :) Так код чуточку понятнее становится

viktoriia-fomina · 2023-03-11T14:39:36Z

HW1_BWT/HW1_BWT/Program.cs

@@ -0,0 +1,58 @@
+using System;


Нужна пустая строка после using

viktoriia-fomina · 2023-03-11T14:46:47Z

HW1_BWT/HW1_BWT/Program.cs

+            var sequence = Console.ReadLine();
+            if (sequence == null)
+            {
+                Console.WriteLine("Empty string");
+                return;
+            }


Здесь пользователь мог ввести, например, Ctrl + Z. В результате такого ввода Console.ReadLine вернет null. Еще пользователь мог нажать Enter, тогда из Console.ReadLine вернется пустая строка. Для пустой строки вы выполняете преобразование и даже выдаете position! То есть ваш вывод для этого случая некорректен. Поправьте, пожалуйста :)

viktoriia-fomina · 2023-03-11T14:50:24Z

HW1_BWT/HW1_BWT/Program.cs

+            sequence = Console.ReadLine();
+            if (sequence == null)
+            {
+                Console.WriteLine("Empty string");
+                return;
+            }


Здесь то же самое. Пользователь мог нажать Enter, Console.ReadLine вернула пустую строку. То есть можно в этот if еще и проверку на пустую строку добавить

AnNyiiik added 6 commits February 20, 2023 08:33

added methods Encode & ComparePermutations & InsertionSort to class BWT

8b2d8b0

changed

8cacd79

fixed method encode

6428303

added test for Encode method

77f11b3

added method decode & getCardinality & getAlphabetPosition & GetAlpha…

e0bef7b

…beticFrequencySequence & SortCharacters in BWT class

added test for decode

f9c400c

viktoriia-fomina suggested changes Feb 23, 2023

View reviewed changes

AnNyiiik added 3 commits February 24, 2023 22:43

changed .NET version, added second arg to decode and added +1 ret val…

db9b445

… to encode, fixed test

HW1_BWT/HW1_BWT/Program.cs

d5d1143

added input output and fixed test encode

HW1_BWT/HW1_BWT/Program.cs

dd46313

added input output

viktoriia-fomina suggested changes Mar 2, 2023

View reviewed changes

AnNyiiik added 2 commits March 5, 2023 16:47

added some adjustments

057250e

added var

f5ed9fe

viktoriia-fomina reviewed Mar 11, 2023

View reviewed changes

added check for null-reference and empty string

314b3a5

Conversation

AnNyiiik commented Feb 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants