LINUX.ORG.RU

[vala][srt2tmx] Покритикуйте код

 ,


0

1

Осваиваю vala. Без практической задачи мне программировать не интересно, поэтому реализовал то, в чём у меня сейчас была потребность.

Программа читает два набора .srt файлов на двух языках, и на основе сопоставления временных меток создаёт файл памяти перевода (translation memory) в формате tmx.

Программа работает, своё дело сделала, но мне интересно, что можно было написать проще/эффективнее/понятнее (например, мне не нравится, как организован двумерный динамический массив для имён файлов).

Много кода:

struct TextEvent
    {
    double begin;
    double end;
    string text;
    }

struct TextPair
    {
    string orig;
    string trans;
    }

struct StringArray
    {
    string[] sa;
    }

int main(string[] args)
    {
    string[] lang_codes={}; /* Languages */
    StringArray[] lang_data={}; /* Source file names */
    string out_file=null; /* Output file name */

    int file_i=0; /* Source file number */

    /* Process command line arguments */
    for(int i=1; i < args.length; ++i)
        {
        if(args[i].has_prefix("-"))
            {
            /* It's an option */
            string opt=args[i].substring(1);
            if("o" == opt)
                {
                /* Output file name */
                ++i;
                if(i < args.length)
                    out_file=args[i];
                }
            else if(2 == opt.length)
                {
                /* Language code */
                lang_codes+=opt.up();
                file_i=0;
                }
            else
                {
                /* Some shit */
                stderr.printf("Invalid option: %s\n", args[i]);
                return 1;
                }
            }
        else
            {
            /* It's a file name */
            if(0 == lang_codes.length)
                {
                /* Language code must go first */
                stderr.printf("Language undefined!\n");
                return 1;
                }

            if(lang_data.length <= file_i)
                {
                /* Allocate new set of files */
                lang_data.resize(file_i + 1);
                lang_data[file_i].sa={};
                }

            if(lang_data[file_i].sa.length < lang_codes.length)
                /* Allocate space for file name */
                lang_data[file_i].sa.resize(lang_codes.length);

            /* Store file name */
            lang_data[file_i].sa[lang_codes.length-1]=args[i];
            ++file_i;
            }
        }

    /* Print source file names */
    for(int lang_i=0; lang_i < lang_codes.length; ++lang_i)
        {
        stdout.printf("%s files:\n", lang_codes[lang_i]);
        foreach(var ld in lang_data)
            {
            if(null == ld.sa[lang_i])
                {
                stderr.printf("File lists mismatch!\n");
                return 1;
                }
            stdout.printf("  %s\n", ld.sa[lang_i]);
            }
        }

    if(lang_codes.length != 2)
        {
        stderr.printf("There must be 2 input languages!\n");
        return 1;
        }

    if(null == out_file)
        {
        stderr.printf("Missing output file name!\n");
        return 1;
        }

    TextPair[] pairs={}; /* Translations */

    /* Process file sets */
    foreach(var ld in lang_data)
        {
        if(null == ld.sa[0] || null == ld.sa[1])
            {
            stderr.printf("File lists mismatch!\n");
            return 1;
            }

        stdout.printf("Processing:\n  %s\n  %s\n", ld.sa[0], ld.sa[1]);

        try
            {
            /* Read, parse and match */
            match_events(ref pairs, ld.sa);
            }
        catch(FileError e)
            {
            stderr.printf("%s\n", e.message);
            return 1;
            }

        }

    /* Write XML */
    stdout.printf("Writing: %s\n", out_file);
    write_pairs(out_file, pairs, lang_codes);

    return 0;
    }

/* Process file set */
void match_events(ref TextPair[] pairs, string[] file_set) throws FileError
    {
    TextEvent[] orig, trans; /* Parsed srt data */
    int i_trans=0, i_orig=0; /* String numbers */
    double o_begin, o_end, o_mid; /* Original time interval */
    double t_begin, t_end, t_mid; /* Translated time interval */

    /* Read and parse files */
    orig=parse_srt(file_set[0]);
    trans=parse_srt(file_set[1]);

    /* Process text events */
    while(i_orig < orig.length && i_trans < trans.length)
        {
        /* Calculate original time interval */
        o_begin=orig[i_orig].begin;
        o_end=orig[i_orig].end;
        o_mid=(o_begin + o_end) / 2.0;
        /* Calculate transalted time interval */
        t_begin=trans[i_trans].begin;
        t_end=trans[i_trans].end;
        t_mid=(t_begin + t_end) / 2.0;
        /* Test for interval match */
        if(t_mid > o_begin && t_mid < o_end || o_mid > t_begin && o_mid < t_end)
            {
            /* Store matched text */
            push_pair(ref pairs, orig[i_orig].text, trans[i_trans].text);
            ++i_trans;
            ++i_orig;
            /* Concatenate overlapping events */
            do
                {
                while(i_trans < trans.length)
                    {
                    t_mid=(trans[i_trans].begin + trans[i_trans].end) / 2.0;
                    if(t_mid < o_begin || t_mid > o_end)
                        break;
                    pairs[pairs.length-1].trans+=" " + trans[i_trans].text;
                    t_end=trans[i_trans].end;
                    ++i_trans;
                    }
                while(i_orig < orig.length)
                    {
                    o_mid=(orig[i_orig].begin + orig[i_orig].end) / 2.0;
                    if(o_mid < t_begin || o_mid > t_end)
                        break;
                    pairs[pairs.length-1].orig+=" " + orig[i_orig].text;
                    o_end=orig[i_orig].end;
                    ++i_orig;
                    }
                }
            while(t_mid > o_begin && t_mid < o_end && i_trans < trans.length);
            }
        else
            {
            /* No matching event */
            if(o_mid < t_mid)
                {
                /* Skip original */
                /*push_pair(ref pairs, orig[i_orig].text, "");*/
                ++i_orig;
                }
            else
                {
                /* Skip translated */
                /*push_pair(ref pairs, "", trans[i_trans].text);*/
                ++i_trans;
                }
            }
        }
    }

/* Append text pair to array */
void push_pair(ref TextPair[] pairs, string t1, string t2)
    {
    pairs.resize(pairs.length+1);
    pairs[pairs.length-1].orig=t1;
    pairs[pairs.length-1].trans=t2;
    }

/* Read and parse srt file */
TextEvent[] parse_srt(string file_name) throws FileError 
    {
    TextEvent[] parsed={};
    string file_text;

    /* Read file */
    FileUtils.get_contents(file_name, out file_text, null);

    /* Parse text */
    foreach(var srt_group in file_text.replace("\r", "").split("\n\n"))
        {
        var srt_part=srt_group.strip().split("\n", 3);
        if(srt_part.length<3)
            break;
        var time_text=srt_part[1].split("-->", 2);
        parsed.resize(parsed.length+1);
        parsed[parsed.length-1].begin=parse_time_value(time_text[0]);
        parsed[parsed.length-1].end=parse_time_value(time_text[1]);
        parsed[parsed.length-1].text=srt_part[2].replace("\n", " ");
        }

    return parsed;
    }

/* Convert srt time to double */
double parse_time_value(string time_text)
    {
    double time_value;

    var time_parts=time_text.strip().replace(",", ".").split(":", 3);
    time_value=double.parse(time_parts[2]);
    time_value+=double.parse(time_parts[1])*60.0;
    time_value+=double.parse(time_parts[0])*3600.0;

    return time_value;
    }

/* Write XML file */
void write_pairs(string file_name, TextPair[] pairs, string[] lang_codes)
    {
    var xml=new Xml.TextWriter.filename(file_name);

    xml.set_indent(true);
    xml.start_document();
    xml.start_element("tmx");
        xml.write_attribute("version", "1.4");
        xml.write_attribute("xmlns", "http://www.localization.org/tmx14");
        xml.start_element("header");
            xml.write_attribute("creationtool", "srt2tmx");
            xml.write_attribute("creationtoolversion", "0.1");
            xml.write_attribute("datatype", "PlainText");
            xml.write_attribute("segtype", "sentence");
            xml.write_attribute("adminlang", "en-us");
            xml.write_attribute("srclang", lang_codes[0]);
            xml.write_attribute("o-tmf", "srt2tmx");
        xml.end_element();
        xml.start_element("body");
    foreach(var p in pairs)
        {
            xml.start_element("tu");
                xml.start_element("tuv");
                    xml.write_attribute("xml:lang", lang_codes[0]);
                    xml.write_element("seg", p.orig);
                xml.end_element();
                xml.start_element("tuv");
                    xml.write_attribute("xml:lang", lang_codes[1]);
                    xml.write_element("seg", p.trans);
                xml.end_element();
            xml.end_element();
        }
        xml.end_element();
    xml.end_element();
    xml.end_document();
    xml.flush();
    }

★★

Последнее исправление: CYB3R (всего исправлений: 1)

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

Чем же он тебе не нравится?
//Код слишком длинен, поэтому не читал :)

Bad_ptr ★★★★★
()

Мне, например, не нравится, что здесь есть несколько длинных функций с большими уровнями вложенности. В подвластном мне коде я всегда стараюсь избавляться от подобных конструкций, используя стандартные приёмы Composed Method, Extract Class (более актуально для жавки, конечно) и прочие.

Попробуй поставить себе ограничение на размер функций (например, «не более 10 строчек») и проведи соответствующие рефакторинги кода, чтобы вписаться в эти ограничения. Весьма полезное упражнение ;)

Zloddey
()
Ответ на: комментарий от Bad_ptr

Чем же он тебе не нравится?

Тем, что пришлось завести отдельный тип для вложенных массивов:

struct StringArray
    {
    string[] sa;
    }

StringArray[] lang_data={}; /* Source file names */

/* Store file name */
            lang_data[file_i].sa[lang_codes.length-1]=args[i];

Сделать массив массивов проще у меня не получилось.

cdslow ★★
() автор топика
Ответ на: CodingStyle от cdslow

тогда хотя бы в стиле K&R пиши, а то такие отступы вызывают тошноту и желание увивать >_<

anonymous
()
Ответ на: комментарий от Zloddey

Теоретически, я согласен, длинные функции это плохо. В данном случае можно было бы разбить main() на части, но особого смысла я в этом не вижу.

cdslow ★★
() автор топика
Ответ на: комментарий от anonymous

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

Да, мне нравится именно такой стиль отступов.

cdslow ★★
() автор топика
Ответ на: CodingStyle от cdslow

Если буду писать код для ядра, приму во внимание.

Там вообще есть полезные вещи, безотносительно ядра, - про K&R и разные полезные мелочи форматирования, про именование переменных, про то что функции нужно делать короче.

quasimoto ★★★★
()
Ответ на: комментарий от cdslow

Да, мне нравится именно такой стиль отступов.

Какие чудеса на свете бывают.

geekless ★★
()

Имхо, эту кашу без пробелов, со «странными» (мягко говоря) отступами (даже внутри блоков уровни отступов «пляшут») читать вообще невозможно.

OldFatMan
()

Относительно стиля: отделяй пробелами любые операторы, в том числе и «=», ставь пробел после if, и используй больше пустых строк.

За скобочки зачёт. Давно не видел стиля Уайтсмитса.

stolz
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.