Skip to content

Homework final 2 - #13

Open
akvise wants to merge 2 commits into
masterfrom
homework_final_2
Open

Homework final 2#13
akvise wants to merge 2 commits into
masterfrom
homework_final_2

Conversation

@akvise

@akvise akvise commented Aug 31, 2021

Copy link
Copy Markdown
Owner

No description provided.

@dimonl dimonl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any tests in app, Whole app is written in non testable style.
I didn't see any containerisation and automatisation of deploying
By the way - It seems . At least this app will work, but it needs some improvements

var conn net.Conn

/// connect with telegram bot
token, err := ReadToken("./telegram_token.json")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoded parameters is bad practice

ErrFunc(err)

updateConfig := tgbotapi.NewUpdate(0)
updateConfig.Timeout = 60

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same thing - hardcoded parameters is evil.


for update := range updates {
if update.Message != nil {
conn, _ = net.Dial("tcp", "127.0.0.1:8080")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again


id := update.Message.Chat.ID

if len(update.Message.Text) > 100 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you limiting message here?

Comment thread lesson_final_2.0/server/db.go
default:
message = "incorrect request"
}
conn.Write([]byte(message + "\v"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unhandled error



func MakeUrl(city string)(string, error){
KeyWeather, err := ReadToken("./weather_token.json")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcode

return "", err
}

URL := "https://api.openweathermap.org/data/2.5/weather?q=" +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

}

cityStr := strings.Replace(request, "/city ", "", 1)
url, _ := MakeUrl(cityStr)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you declare func as (result, error) - why you don't handle error?

}

InitDB()
err = AddDBRequest(cityStr, message)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't extract different layers in application. All in one file - handle requests, app logic, work with db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants