Код ревью

    После публикации моей статьи на habr, я начал получать огромное количество писем на разные контакты (не только в ЛС хабра) и даже на те контакты которые я нигде не указывал(но найти их не сложно). Некоторые люди начали использовать cine и писать на нём программы(просто поэкспериментировать и по изучать язык, даже не смотря на то, что туториалов ещё маловато). И вот сегодня я получаю в ЛС хабра сообщение, в котором человек написал на cine обработчик комментариев для своего сайта(тоже ради прикола). Я решил сделать код ревью его кода и написать статью для тех, кто уже начал писать на cine. Я буду приводить код человека который мне написал, а затем напишу что не так и затем код который я написал. Так же я залил на google drive оба проекта.

func fileGetContents(fileName)
    rules
        final = fileName == [UInt8]
        result = [UInt8]
    fileBox := fileName.loadFile()
    result = fileBox[].convertTo([UInt8]).asString()

    С помощью ":=" объявляются переменные, но fileBox не изменяется, поэтому fileBox нужно объявить как константу ".=", здесь это не критично, но если всегда всё объявлять через переменную, то выработается такая привычка. А возможен случай когда программист объявляет переменную, думая что она не будет изменятся, а какая либо функция возьмет и изменит её, с константой такая ситуация - ошибка компиляции. Да и при чтении кода это удобно, так как просматривая чужой код, другой человек будет понимать, что может изменится в процессе работы, а что нет.

func fileGetContents(fileName)
    rules
        final = fileName == [UInt8]
        result = [UInt8]
    fileBox .= fileName.loadFile()
    result = fileBox[].convertTo([UInt8]).asString()

---

func isHexCode(char1, char2)
    rules
        final = char1 == UInt8 & char2 == UInt8
        result = Bool
    if char1 < '0' || char1 > '9'
        then if char1 < 'a' || char1 > 'f'
            then if char1 < 'A' || char1 > 'F'
                then return false
    if char2 < '0' || char2 > '9'
        then if char2 < 'a' || char2 > 'f'
            then if char2 < 'A' || char2 > 'F'
                then return false
    return true

    Тут есть над чем поработать. Во первых, в именах функций последним символом допускается использовать ? и !. Смысл такой ? - функция что то узнаёт, ! - функция что то утверждает и если это не так, выводит ошибку. Поэтому не "isHexCode" а "hexCode?". Во вторых, в стандартном модуле есть файл Char.cine в котором содержаться функции для работы с UInt8 как с символом строки. В этом файле есть функция "digit?" которая  узнаёт, является ли символ цифрой. В третьих, UInt8 входит в группу Ordered члены которой, поддерживают функции для сравнения на больше-меньше, также все члены группы Ordered должны входить в группу Equal, члены которой могут сравниваться на равенство. Есть такая функция "inRange?" которая работает с типами входящими в группу Ordered, эта функция выясняет находится ли элемент в указанном диапозоне. Навсякий случай скажу, что у этой функции есть особенность - она включает конечное значение в диапозон, во всех других местах в cine конечный элемент в диапозон не включается. В четвертых, операторы "||" и "&&" - ленивые, т.е. если по левому операнду будет понятен результат, то правый операнд не будет вычислен, поэтому if'ы не нужны.

func hexCode?(char1, char2)
    rules
        final = char1 == UInt8 & char2 == UInt8
        result = Bool
    result = (char1.digit?() || char1.inRange?('a', 'f') || char1.inRange?('A', 'F')) && {
        (char2.digit?() || char2.inRange?('a', 'f') || char2.inRange?('A', 'F'))
    }

---

func hexToByte(char1, char2)
    rules
        final = char1 == UInt8 & char2 == UInt8
        result = UInt8
    include
        #include <stdio.h>
    C
        char str[2];
        str[0] = char1_;
        str[1] = char2_;
        result_ = (char)strtol(str, NULL, 16);

    Здесь не нужна вставка на Си и уж тем более не нужен вызов функции strtol. Но вначале хочу представить встроенный в язык оператор select, он работает как функция и принимает три аргумента, первый должен быть типа Bool, если первый аргумент равен true то select возвращает значение второго аргумента, иначе третьего. Только выражение возвращаемого аргумента вычисляется, оставшийся игнорируется. Второй и третий аргумент должны иметь одинаковый тип.

func hexToByte(char1, char2)
    rules
        final = char1 == UInt8 & char2 == UInt8
        result = UInt8
    bits0_3 .= char1 - select(char1.digit?(), '0', 'a' - 10.convertTo(UInt8))
    bits4_7 .= (char2 - select(char2.digit?(), '0', 'a' - 10.convertTo(UInt8))) << 4
    result = bits0_3 | bits4_7

---

func urldecode(urlEncodedString)
    rules
        final = urlEncodedString == [UInt8]
        result = [UInt8]
    length := urlEncodedString.getLength()
    result = ""
    for pos := 0; pos < length; pos++
        char := urlEncodedString[pos]
        if char == '%' && pos < length - 2
            then
                char1 := urlEncodedString[pos+1]
                char2 := urlEncodedString[pos+2]
                if isHexCode(char1, char2)
                    then
                        result += hexToByte(char1, char2)
                        pos += 2
                        continue
            else if char == '+'
                then result += ' '
                else result += char

    Почему то название функции не в camel case, хотя в других именах всё нормально. pos += 2 и continue можно заменить на pos++. Ну и несколько проблем о которых я писал в других примерах.

func urlDecode(urlEncodedString)
    rules
        final = urlEncodedString == [UInt8]
        result = [UInt8]
    result = ""
    length .= urlEncodedString.getLength()
    for pos := 0; pos < length; pos++
        char .= urlEncodedString[pos]
        if char == '%' && pos < length - 2
            then
                char1 .= urlEncodedString[pos + 1]
                char2 .= urlEncodedString[pos + 2]
                if hexCode?(char1, char2)
                    then
                        result += hexToByte(char1, char2)
                        pos++
            else result += select(char == '+', ' ', char)

---

func parseUrlData(urlData)
    rules
        final = urlData == [UInt8]
        result = Map([UInt8], [UInt8])
    pairs := urlData.split('&')
    for n := 0; n < pairs.getLength(); n++
        pair := pairs[n].split('=')
        key := urldecode(pair[0])
        value := ""
        if pair.getLength() > 1
            then value = urldecode(pair[1])
        add(result, key, value)

     URL - аббревиатура, все символы должны быть в одном регистре. В cine есть оператор parse который может разбирать объекты некоторых типов поэлементно, для этого должны быть определены несколько функций, которые будут поддерживать указанный тип. В будущих статьях я расскажу о parse подробней(там много нюансов). parse можно использовать для типов Slice, Map, Set. Синтаксис parse key, value < - parsedObject, а дальше как в for. Для слайсов key это индекс элемента, а value сам элемент.

func parseURLData(urlData)
    rules
        final = urlData == [UInt8]
        result = Map([UInt8], [UInt8])
    parse _, pair <- urlData.split('&')
        key_value .= pair.split('=')
        key .= urlDecode(key_value[0])
        value .= select(key_value.getLength() > 1, urlDecode(key_value[1]), "")
        result:add(key, value)

---

func getTime()
    rules
        result = [UInt8]
    include
        #include <time.h>
    dateBox := Box([UInt8])
    C
        time_t timer;
        char buffer[26];
        struct tm* tm_info;
        time(&timer);
        tm_info = localtime(&timer);
        strftime(buffer, 26, "%d.%m.%Y %H:%M:%S", tm_info);
        char date[26];
        //date[10] = '\0';
        strcpy(date, buffer);
        dateBox_.private_item_.length = strlen(date);
        dateBox_.private_item_.capacity = dateBox_.private_item_.length;
        dateBox_.private_item_.items = (uint8_t*)date;
        dateBox_.private_item_.isString = true;
        dateBox_.private_haveItem_ = true;
    result = dateBox.getItem() + "" // Иначе баг

    За попытку использования такой Си вставки моё почтение, но это дело крайне рискованное. Здесь не нужен Box, он вообще для других целей, ну и раз задели Box, [] -это не относится к слайсу, это синтаксический сахар для функции getItem, т.е. dateBox.getItem() тоже самое, что и dateBox[]. Баг связан с тем, что date выделен на стеке, а то что выделено на стеке, не может покинуть функцию, а при добавлении пустой строки он исчезает из-за того, что при соединении двух строк результирующая строка хранится в куче, а то что хранится в куче, может покинуть функцию. Для использования strcpy нужно подключить string.h.

func getTime()
    rules
        result = [UInt8]
    include
        #include <time.h>
        #include <string.h>
    length := 0
    C
        time_t timer;
        char buffer[26];
        struct tm* tm_info;
        time(&timer);
        tm_info = localtime(&timer);
        length_ = strftime(buffer, 26, "%d.%m.%Y %H:%M:%S", tm_info);
    result = [UInt8].init(length).asString()
    C
        memcpy(result_.items, buffer, length_);

---

proc main()
    print("Content-Type: text/html; charset=utf8\n\n")

    //requestUri := getEnvironmentVariable("REQUEST_URI")
    //requestUri := "/board.cgi?action=addComment&nick=Pascal&comment=Preved+Medved"
    requestUri := "/board.cgi"
    pair := requestUri.split('?')
    path := pair[0]
    queryString := ""
    if pair.getLength() > 1
        then queryString = pair[1]
    getParameters := parseUrlData(queryString)
    action := getItem(getParameters, "action").getItem()
    commentsPath := "/tmp/comments.txt"

    if action == "addComment"
        then
            nick := getItem(getParameters, "nick").getItem()
            comment := getItem(getParameters, "comment").getItem()
            time := getTime()
            if nick == ""
                then
                    print("Не указан ник\n")
                    exit()
            if comment == ""
                then
                    print("Пустой текст комментария\n")
                    exit()
            comments := "[" + time + "] " + nick + "\n" + comment + "\n\n"
            comments += fileGetContents(commentsPath)
            ok := filePutContents(commentsPath, comments)
            print("Комментарий добавлен\n")
        else
            print(fileGetContents(commentsPath))

    Куча проблем о которых я писал ранее. Переменная path объявлена, но не используется. getParametrs - очень плохое имя для переменной. Извлечение из пустого бокса - неопределенное поведение(при включенном дебаге, было бы куча предупреждений). Для соединения нескольких строк, есть функция join(она гораздо эффективнее). Если результат функции не нужен можно присвоить его знаку подчеркивания, а в данном случае функцию filePutContents нужно было объявить как процедуру. Если в else или then одна строка, её не обязательно писать с новой строки, можно просто написать это после then или else.

proc main()
    print("Content-Type: text/html; charset=utf8\n\n")

    //requestUri := getEnvironmentVariable("REQUEST_URI")
    //requestUri := "/board.cgi?action=addComment&nick=Pascal&comment=Preved+Medved"
    requestUri .= "/board.cgi"
    pair .= requestUri.split('?')
    path .= pair[0]
    queryString .= select(pair.getLength() > 1, pair[1], "")
    parameters .= parseURLData(queryString)
    action .= parameters["action"][]
    commentsPath .= "/tmp/comments.txt"

    if action == "addComment"
        then
            nickBox .= parameters["nick"]
            commentBox .= parameters["comment"]
            time .= getTime()
            if nickBox.empty?()
                then
                    print("Не указан ник\n")
                    exit()
            if commentBox.empty?()
                then
                    print("Пустой текст комментария\n")
                    exit()
            comments .= "[".join(time, "] ", nickBox[], "\n", commentBox[], "\n\n", commentsPath.fileGetContents())
            _ = filePutContents(commentsPath, comments)
            print("Комментарий добавлен\n")
        else print(commentsPath.fileGetContents())

---

P.S. Я вообще не вникал, в то что делает программа, я исправил только явные проблемы. В коде я видел очень мало анализа на предмет ошибок. Если функция возвращает Box, нужно проверять пустой ли он или нет(функции empty? и item?), извлечение из пустого бокса - неопределенное поведение. Такие действия, нужно делать, только если на 100% есть уверенность, что Box не пустой.