New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2sem1.2 #34
2sem1.2 #34
Conversation
Не бейте сильно тапком, но обратный оптимизированный чего-то не работает. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прямое преобразование, как я понимаю, то, что называется "наивным", с явным построением таблицы вращений, оно на балл "дешевле" "настоящего" BWT. Обратное действительно не работает :) Там temp неправильно заполняется вроде, не строит перестановку. Надо поотлаживаться (в крайнем случае, на бумажке написать, что должно быть, и пошагово пройтись, смотря на значения temp, line и alphabetOfString).
И ещё это может быть несколько преждевременно, но тесты нужны :)
@@ -141,6 +141,8 @@ | |||
<ItemGroup> | |||
<ClCompile Include="Source.c"> | |||
<SDLCheck Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">false</SDLCheck> | |||
<SDLCheck Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">false</SDLCheck> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тоже какие-то лишние коммиты в пуллреквесте :) Замерджили бы все C++ные изменения в мастер, да и всё
{ | ||
public static void SwapStrings(char[,] transpositionTable, int size, int i, int j) | ||
{ | ||
char temp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
temp вне цикла не нужен, следовательно, и объявлять его надо на 13-й строчке, сразу int temp = transpositionTable[i, t];
} | ||
} | ||
return 1; | ||
} |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
||
public static int CompareStrings(char[,] transpositionTable, int size, int i, int j) | ||
{ | ||
for(int t = 0; t < size; ++t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Пробела после for не хватает
{ | ||
for(int t = 0; t < size; ++t) | ||
{ | ||
if(transpositionTable[i, t].CompareTo(transpositionTable[j, t]) < 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И после if. Интересно, Visual Studio же обычно сама форматирует код правильно :)
И ниже надо поправить
{ | ||
class Program | ||
{ | ||
public static void SwapStrings(char[,] transpositionTable, int size, int i, int j) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
На самом деле, все методы, которые извне не должны быть доступны, должны быть private (это типа функций в .c-файлах, тогда как public --- это типа функций в .h-никах)
|
||
public static string GetAlphabetString(char[] line) | ||
{ | ||
int alphabetSymbols = line.Distinct().Count(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мне кажется, нигде не потребовалось
int sum = 0; | ||
for(int i = 0; i < countOfSymbols.Length; i++) | ||
{ | ||
sum = sum + countOfSymbols[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sum += countOfSymbols[i];
{ | ||
Console.Write(result[i]); | ||
} | ||
char[] res2 = ReverseBWT(result, transpositionTable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этак Вы предлагаете помимо строки передавать ещё матрицу? Не, хочется, чтобы BWT возвращал строку и ReverseBWT только строку принимал. Иначе получится архиватор, который раздувает файл в N^2 раз, такой никому не нужен :)
for (int i = 0; i < line.Length + 1; ++i) | ||
{ | ||
Console.Write(result[i]); | ||
} |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теперь алгоритмически вроде всё ок, но качество кода ещё надо улучшать.
if(transpositionTable[i, t].CompareTo(transpositionTable[j, t]) < 0) | ||
{ | ||
return 1; | ||
} else if (transpositionTable[i, t].CompareTo(transpositionTable[j, t]) > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
char[,] transpositionTable = new char[size, size]; | ||
|
||
var result = BWT(line, transpositionTable).Item1; | ||
var numberOfString = BWT(line, transpositionTable).Item2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теперь нет нужды тут transpositionTable создавать, с этим прекрасно справится сам BWT. Всё, что можно не передавать в функцию, нельзя передавать в функцию.
|
||
var result = BWT(line, transpositionTable).Item1; | ||
var numberOfString = BWT(line, transpositionTable).Item2; | ||
var resultString = new String(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
То же касается возврата массива символов, а не строки. BWT нетрудно преобразовать Item1 в строку самостоятельно, зачем заставлять вызывающего это делать? Абстракция должна быть удобна для использования, а не заставлять выполнять пляски с бубном при каждом вызове.
var result = BWT(line, transpositionTable).Item1; | ||
var numberOfString = BWT(line, transpositionTable).Item2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Угадайте, как сделать этот код в два раза быстрее. Это можно сделать и только используя Ваши текущие знания, но в C# ещё и декомпозиция есть: var (result, numberOfString) = ...
Console.WriteLine("Resulting string: " + resultString); | ||
|
||
char[] res2 = ReverseBWT(result, numberOfString); | ||
var resultString2 = new String(res2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут тоже, может, имеет смысл возвращать сразу строку и сразу без стоп-символа
if (oldString1.Remove(string1.Length).CompareTo(string1) != 0) | ||
{ | ||
return false; | ||
} | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
За такое даже в первом семестре отчисляли :)
var numberOfString = BWT(string1, transpositionTable).Item2; | ||
|
||
|
||
if (result.CompareTo("annb$aa") != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Проверять строки на равенство можно и ==
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ага, теперь всё ок
if (transpositionTable[i, t] < (transpositionTable[j, t])) | ||
{ | ||
return 1; | ||
} else if (transpositionTable[i, t] > (transpositionTable[j, t])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Правильнее
}
else if (transpositionTable[i, t] > (transpositionTable[j, t]))
{
|
||
string oldString1 = ReverseBWT("annb$aa", numberOfString); | ||
|
||
return (oldString1 == string1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут скобки не нужны
new solution added