3
\$\begingroup\$

I'm a freshman student.

What does my program solve?

This is actually a bot for reddit, and there are language learning subreddits on that website, sometimes when discussing something with people, we may need to show declension or conjugation tables to them, instead of sending a link, this bot will help you to comment it automatically.

What does my code do in my opinion?

It only works for lithuanian language, there is a website for declensions or conjugations for lithuanian language, and my program goes to that website, scrapes the html table off of it, makes it form into a table, and then replies in the comments.

Why do I want a review?

Well actually, this was supposed to be a learning project, I want to know the weaknesses in my code, stuff I've done wrong or the parts that I could code it better. It does work for sure, but I dont think that's enough to be a good programmer.

Is my code clean enough? Is my code DRY enough or not? Is it slow? Anything broken or wrong? Any suggestions? Did I seperate the folders correctly? You can find all the source code here on github This is how a comment looks like

This is how my directory looks like:

>src
    >config
        >lietuvos-config.js
    >service
        >bot.js
    >util
        >extract-word.js
        >get-table.js
        >pretty-stringified.js
    >app.js
>package.lock
>readme.md
>package.json

extract-word.js

const extractor = function(query){
    const pattern = /<[a-zA-Z]+>/; //only works with latin letters for now, to be updated
    const stringToSplit = query; //make it case insensitive
    const extractedWord= stringToSplit.match(pattern)
    extractedWord[0] = extractedWord[0].replace("<","")
    extractedWord[0] = extractedWord[0].replace(">","")
    return extractedWord
}

module.exports={
    extractor: extractor
}

get-table.js

var scraper = require('table-scraper');
const {prettifier} = require('../util/pretty-stringified')

const tableOfContent = function(query){
        return scraper.get('https://morfologija.lietuviuzodynas.lt/zodzio-formos/'+query)
                .then(function(tableData){ 
                    console.log(tableData)
                    return prettifier(tableData).toString() //JSON.stringify(tableData)
                }) 
                .catch((error)=>{
                    return "error"
                }) 
}

module.exports = {
    tableOfContent: tableOfContent
}

pretty-stringified.js

const tablemark = require('tablemark')

const prettifier = function(query){
    const decider = nounOrVerb(query);
    if(decider == "noun"){
        return declineNouns(query)
    }else if(decider == "verb"){
        return conjugateVerbs(query)
    }else if(decider == "adjective"){
        return declineAdjectives(query)
    }
}
const convertArrays = function(array){
    var newArr = [];
    for(var i = 0; i < array.length; i++){
    newArr = newArr.concat(array[i]);
    }
    return newArr
}
const declineNouns = function (query){
    const newArr = convertArrays(query)
    var tidiedArr = []
    for (var i = 0; i < newArr.length; i++) {
        tidiedArr.push(newArr[i]["\Š\."])
        tidiedArr.push(newArr[i].Vienaskaita)
    }
    var redditTable = tablemark([
        {Form:"**V.**",Vienaskaita:tidiedArr[0],Daugiskaita:tidiedArr[1]},
        {Form:"**K.**",Vienaskaita:tidiedArr[2],Daugiskaita:tidiedArr[3]},
        {Form:"**K.**",Vienaskaita:tidiedArr[4],Daugiskaita:tidiedArr[5]},
        {Form:"**G.**",Vienaskaita:tidiedArr[6],Daugiskaita:tidiedArr[7]},
        {Form:"**Įn.**",Vienaskaita:tidiedArr[8],Daugiskaita:tidiedArr[9]},
        {Form:"**Vt.**",Vienaskaita:tidiedArr[10],Daugiskaita:tidiedArr[11]},
        {Form:"**Š.**",Vienaskaita:tidiedArr[12],Daugiskaita:tidiedArr[13]},
    ])
    return redditTable;
}
const conjugateVerbs = function(query){
    const newArr = convertArrays(query)
    var tidiedArr = []
    for (var i = 0; i < newArr.length; i++) {
        tidiedArr.push(newArr[i]["Jie/jos"])
        tidiedArr.push(newArr[i]["Esamasis laikas"])
        tidiedArr.push(newArr[i]["Būtasis kartinis laikas"])
        tidiedArr.push(newArr[i]["Būtasis dažninis"])
    }
    var redditTable = tablemark([
        {Įvardis:"**Aš**","**Esamasis laikas**":tidiedArr[0],"**Būtasis kartinis laikas**":tidiedArr[1],"**Būtasis dažninis**":tidiedArr[2],"**Būsimasis laikas**":tidiedArr[3]},
        {Įvardis:"**Tu**","**Esamasis laikas**":tidiedArr[4],"**Būtasis kartinis laikas**":tidiedArr[5],"**Būtasis dažninis**":tidiedArr[6],"**Būsimasis laikas**":tidiedArr[7]},
        {Įvardis:"**Jis/ji**","**Esamasis laikas**":tidiedArr[8],"**Būtasis kartinis laikas**":tidiedArr[9],"**Būtasis dažninis**":tidiedArr[10],"**Būsimasis laikas**":tidiedArr[11]},
        {Įvardis:"**Mes**","**Esamasis laikas**":tidiedArr[12],"**Būtasis kartinis laikas**":tidiedArr[13],"**Būtasis dažninis**":tidiedArr[14],"**Būsimasis laikas**":tidiedArr[15]},
        {Įvardis:"**Jūs**","**Esamasis laikas**":tidiedArr[16],"**Būtasis kartinis laikas**":tidiedArr[17],"**Būtasis dažninis**":tidiedArr[18],"**Būsimasis laikas**":tidiedArr[19]},
        {Įvardis:"**Jie/jos**","**Esamasis laikas**":tidiedArr[20],"**Būtasis kartinis laikas**":tidiedArr[21],"**Būtasis dažninis**":tidiedArr[22],"**Būsimasis laikas**":tidiedArr[23]},
    ])
    return redditTable;
}
const declineAdjectives = function(query){
    const newArr = convertArrays(query)
    var tidiedArr = []
    for (var i = 0; i < newArr.length; i++) {
        tidiedArr.push(newArr[i]["\Š\."])
        tidiedArr.push(newArr[i].Vienaskaita)
        tidiedArr.push(newArr[i].Daugiskaita)
        tidiedArr.push(newArr[i]["Vienaskaita_2"])
    }
    var redditTable = tablemark([
        {Form: "",name:"**Vienaskaita**",name2:"**Daugiskaita**",name3:"**Vienaskaita**",name4:"**Daugiskaita**"},
        {Form:"**V.**",name:tidiedArr[0],name2:tidiedArr[1],name3:tidiedArr[2],name4:tidiedArr[3]},
        {Form:"**K.**",name:tidiedArr[4],name2:tidiedArr[5],name3:tidiedArr[6],name4:tidiedArr[7]},
        {Form:"**K.**",name:tidiedArr[8],name2:tidiedArr[9],name3:tidiedArr[10],name4:tidiedArr[11]},
        {Form:"**G.**",name:tidiedArr[12],name2:tidiedArr[13],name3:tidiedArr[14],name4:tidiedArr[15]},
        {Form:"**Įn.**",name:tidiedArr[16],name2:tidiedArr[17],name3:tidiedArr[18],name4:tidiedArr[19]},
        {Form:"**Vt.**",name:tidiedArr[20],name2:tidiedArr[21],name3:tidiedArr[22],name4:tidiedArr[23]},
        {Form:"**Š.**",name:tidiedArr[24],name2:tidiedArr[25],name3:tidiedArr[26],name4:tidiedArr[27]},
    ],{
        columns:[
            "Form",
            {name: "Vyriškoji giminė"},
            {name:" "},
            {name: "Moteriškoji giminė"},
            {name:" "}
        ]
    })
    return redditTable;
}
const nounOrVerb = function(array){
    try{
        const listOfTable = Object.keys(array[0][0]);
        if(listOfTable[0] == "Jie/jos"){
                return "verb"
        }else if(listOfTable[3] =="Vienaskaita_2"){
                return "adjective"
        }else if(listOfTable[0] == "Š."){
                return "noun"
        }
    }catch{
        return "Error"
    }
   
   
}
module.exports = {
    prettifier:prettifier
}

bot.js

const {comments} = require('../config/lietuvos-config')
const {extractor} = require('../util/extract-word')
const {tableOfContent} = require('../util/get-table')
const BOT_START = Date.now() / 1000;

const canSummon = (msg) => {
    /*if(msg){
        msg.toLowerCase().includes('!inspect');
        //function
        return;
    }
    return*/
    return msg && msg.toLowerCase().includes('!inspect');
};

const commenting =  function(){
    comments.on('item',async (item) => {
        try{
            var replyString = "labas u/"+item.author.name+"! esu robotas ir pateikiu lentelę apie žodžius, veiksmažodžius, būdvardžius."+
                " jei prieš nieko, čia tavo žodžio lentelė. net galite sužinot daugiau čia ^[šaltinis](https://morfologija.lietuviuzodynas.lt/zodzio-formos/"+extractedWord+
                ") \n\n "
                var errorReply= "labas u/"+item.author.name+"! esu bandęs rasti žodį, kurį rašėi, atsiprašau ir dėja, bet negalėjau rasti. "+
                    "gal tas žodis neegzistuoja lietuvių kalboj, rašėi neteisingai - arba yra klaida mano kode.\n šiaip ar taip, galite bandyt rast savarankiškai čia ^[žodynas](https://morfologija.lietuviuzodynas.lt/"
                var replyStringEnder = " \n\n \*\*\* \n ^feel ^free ^to ^report ^bugs ^or ^errors\n ^\[[source-code]\](https://github.com/wulfharth7/lietuvos-robotas) ^| ^\[[buy-me-a-coffee☕]\](https://www.buymeacoffee.com/beriscen)"
            if(item.created_utc < BOT_START) return;
            if(!canSummon(item.body)) return;
            var extractedWord = extractor(item.body)
            tableOfContent(extractedWord).then(function(tableofLog){
                if(tableofLog !== "error"){
                    item.reply(replyString+ tableofLog+replyStringEnder)
                }else{
                    item.reply(errorReply+replyStringEnder)
                }
            })
        }catch(Error){  
            var errorReply= "labas u/"+item.author.name+"! esu bandęs rasti žodį, kurį rašėi, atsiprašau ir dėja, bet negalėjau rasti. "+
                    "gal tas žodis neegzistuoja lietuvių kalboj, rašėi neteisingai - arba yra klaida mano kode.\n\n šiaip ar taip, galite bandyt rast savarankiškai čia ^[žodynas](https://morfologija.lietuviuzodynas.lt/)"
                    item.reply(errorReply+replyStringEnder)
        }
    
});
} 

module.exports={
    commenting: commenting
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

extract-word.js

Well, you're clearly already aware that it only working with latin letters is an issue, but I'll point it out anyway. Since, y'know, it excludes a few letters used in lithuanian.

By the way, the regular expression can be made case-insensitive with the i flag, like const pattern = /<[a-z]+>/i

stringToSplit is a bit redundant - it contains exactly the same content as query, so we may as well operate on query directly

We can clean up the extracting a bit by using capturing groups. If we do const pattern = /<([a-z]+)>/i, the parens define a group, and we can access that group as extractedWord[1]

All in all, that function could be boiled down into a one-liner like return query.match(/<([a-z]+)>/i)[1];. Or perhaps return query.match(/<([a-ząčęėįšųūž]+)>/i)[1]; to add some letters (I hope those are the right ones). More sensible might be to keep the pattern on a separate line, like:

const extractor = function(query){
    const pattern = /<([a-ząčęėįšųūž]+)>/i;
    return query.match(pattern)[1];
}

module.exports={
    extractor: extractor
}

get-table.js

I do wonder if returning "error" in case of failure is the most useful behaviour. Would it not be more convenient to just... let the failure remain a failure, to make it easier for the caller to detect and clean up? Since bot.js already has a catch block that provides an error message, it might be best to just... let it deal with errors from here as well

Then we could have this file looking closer to

var scraper = require('table-scraper');
const {prettifier} = require('../util/pretty-stringified')

const tableOfContent = function(query) {
    return scraper.get('https://morfologija.lietuviuzodynas.lt/zodzio-formos/' + query)
        .then(function(tableData) { 
            console.log(tableData);
            return prettifier(tableData).toString();
        });
}

module.exports = {
    tableOfContent: tableOfContent
}

pretty-stringified.js

Well, for one, the name of nounOrVerb is a bit misleading, since it has three possible results. For now. Maybe one day it might even have a fourth? Let's give it a name that leaves room for expansion. Something like const typeOfWord = getTypeOfWord(query); perhaps?

Alternatively, instead of having a function return a string, and then branching based on what that string is... we could just return the function directly:

const prettify = function(query) {
    const prettifier = choosePrettifier(query);
    return prettifier(query);
}

const choosePrettifier = function(table) {
    try {
        const listOfTable = Object.keys(table[0][0]);
        if (listOfTable[0] == "Jie/jos") {
            return conjugateVerbs;
        } else if (listOfTable[3] == "Vienaskaita_2") {
            return declineAdjectives;
        } else if (listOfTable[0] == "Š.") {
            return declineNouns;
        }
    }
}

We could even call it right there but... I don't know, I think that looks worse somehow. Which might be a sign that there's an even better solution that I'm missing right now.

Moving on to the conjugate/decline functions, they seem to follow a somewhat odd pattern. They take an array containing some manner of structured objects, removes that structure by shoving their fields into a flat array, then re-adds structure by working on individual array indices. That feels a bit roundabout. For example, looking at declineNouns, it seems like something like this should work:

const declineNouns = function(query) {
    const newArr = convertArrays(query);
    const forms = ["**V.**", "**K.**", "**K.**", "**G.**", "**Įn.**", "**Vt.**", "**Š.**"];

    const tidiedArr = [];
    // There's probably some even neater functional way to do this, but I don't remember it right now
    for (var i = 0; i < newArr.length; ++i) {
        tidiedArr.push({Form: forms[i], Vienaskaita: newArr[i]["\Š\."], Daugiskaita: newArr[i].Vienaskaita});
    }

    return tablemark(tidiedArr);
}

Now, I know that Daugiskaita: newArr[i].Vienaskaita part looks a bit questionable to me when a Vienaskaita key also exists, but the old code had Daugiskaita: tidiedArr[1], and tidiedArr[1] was set by tidiedArr.push(newArr[i].Vienaskaita), so I'm gonna assume that it's correct

bot.js

Commented-out code in canSummon should be deleted. Should we need it back for some reason, there's always version control.

errorReply is defined the exact same way twice. I would suggest doing it just once by the start of the function instead. Since it depends on the comment author's username, we could either define it by the start of the callback passed to comments.on, or we could have a "template function" of sorts that just takes a username and spits out the correct error message. I kind of like the latter, but either works.

extractedWord seems to be used to create replyString before it is actually set. Thanks to JS's scoping rules, I wouldn't be entirely surprised if it finds a word, but I would expect it to find a word an earlier commenter asked for instead. We'll probably want to make sure the reply is created after we have all the content that goes into it, whether that be by moving the definition later or by passing it to a function that slots it into a string

We may also be able to save some repetition by having only a single item.reply call towards the end

If we also go with the "throwing an exception instead of returning "error"" idea mentioned earlier, I'd probably do something not too far from the following:

const commenting =  function(){
    let replyString = function(username, extractedWord) {
        return `labas u/${username}! esu robotas ir pateikiu lentelę apie žodžius, veiksmažodžius, būdvardžius.` +
            ` jei prieš nieko, čia tavo žodžio lentelė. net galite sužinot daugiau čia ^[šaltinis](https://morfologija.lietuviuzodynas.lt/zodzio-formos/${extractedWord}")`;
    };

    let errorReply = function(username) {
        return `labas u/${item.author.name}! esu bandęs rasti žodį, kurį rašėi, atsiprašau ir dėja, bet negalėjau rasti. ` +
            "gal tas žodis neegzistuoja lietuvių kalboj, rašėi neteisingai - arba yra klaida mano kode.\n šiaip ar taip, galite bandyt rast savarankiškai čia ^[žodynas](https://morfologija.lietuviuzodynas.lt/";
    }

    let replyStringEnder = " \n\n \*\*\* \n ^feel ^free ^to ^report ^bugs ^or ^errors\n ^\[[source-code]\](https://github.com/wulfharth7/lietuvos-robotas) ^| ^\[[buy-me-a-coffee☕]\](https://www.buymeacoffee.com/beriscen)";
    
    comments.on('item',async (item) => {
        let message;
        try {            
            if(item.created_utc < BOT_START) return;
            if(!canSummon(item.body)) return;
            let extractedWord = extractor(item.body);
            table = await tableOfContent(extractedWord);
            message = replyString(item.author.name) + table;
        } catch(Error) {
            message = errorReply(item.author.name);
        }

        item.reply(message + replyStringEnder);
    });
} 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.