11
\$\begingroup\$

I have built a Twitter clone in Golang, using object orientation principles. I wish to know the design mistakes I have made.

package blade
import (
 "database/sql"
 "fmt"
)
type User struct {
 Name string
 Password string
 *sql.Tx
}
//User is a normal user who can login, register, tweet, retweet, follow and unfollow
func Stream(username string, transaction *sql.Tx) []Tweetmodel {
 rows, err := transaction.Query("Select id, tweet from tweets where username=1ドル", username)
 if err != nil {
 panic(err)
 }
 defer rows.Close()
 tweets := make([]Tweetmodel, 0)
 for rows.Next() {
 var id int
 var msg string
 rows.Scan(&id, &msg)
 tweet := Tweetmodel{id, msg}
 tweets = append(tweets, tweet)
 }
 return tweets
}
func (u *User) Follow(usertofollow User) string {
 name := usertofollow.Name
 var username string
 err := u.QueryRow("SELECT name from users where name=1ドル", name).Scan(&username)
 switch {
 case err == sql.ErrNoRows:
 return "You cannot follow an user who does not exist"
 default:
 if u.alreadyfollowing(name) {
 return "You have already followed this user"
 } else {
 u.Exec("INSERT INTO follow(username, following) VALUES(1,ドル 2ドル)", u.Name, name)
 return fmt.Sprintf("You have successfully followed %v", name)
 }
 }
}
func (u *User) alreadyfollowing(usertofollow string) bool {
 res, _ := u.Query("SELECT * from follow where username=1ドル and following=2ドル", u.Name, usertofollow)
 return (res.Next() == true)
}
func (u User) Login() string {
 var username, password string
 err := u.QueryRow("SELECT name, password FROM users WHERE name=1ドル", u.Name).Scan(&username, &password)
 if err == sql.ErrNoRows {
 return "There is no user with that name, please try again or try registering!"
 }
 if u.Password != password {
 return "Your password is wrong, please try again!"
 }
 return "Welcome to Twitchblade"
}
func (u *User) Register() string {
 var username string
 err := u.QueryRow("SELECT name FROM users WHERE name=1ドル", u.Name).Scan(&username)
 switch {
 case err == sql.ErrNoRows:
 u.Exec("INSERT INTO users(name, password) VALUES(1,ドル 2ドル)", u.Name, u.Password)
 return "Successfully registered"
 default:
 return "User exists with same name.Please try a new username"
 }
}
func (u *User) alreadyretweeted(tweetid int) bool {
 var id int
 err := u.QueryRow("SELECT id from retweets where original_tweet_id = 1ドル and retweeted_by = 2ドル", tweetid, u.Name).Scan(&id)
 return (err != sql.ErrNoRows)
}
func (u *User) iteratedretweet(tweetid int) (bool, int) {
 var id int
 err := u.QueryRow("SELECT original_tweet_id from retweets where retweet_tweet_id = 1ドル", tweetid).Scan(&id)
 return (err != sql.ErrNoRows), id
}
func (u *User) Retweet(tweetid int) (string, int) {
 if u.alreadyretweeted(tweetid) {
 return "You have already retweeted this tweet", tweetid
 } else {
 flag, originalid := u.iteratedretweet(tweetid)
 if flag {
 return u.Retweet(originalid)
 } else {
 var msg, originaluser string
 var id int
 u.QueryRow("select username, tweet from tweets where id=1ドル", tweetid).Scan(&originaluser, &msg)
 _, retweetid := u.Tweet(msg)
 u.QueryRow("INSERT INTO retweets(original_tweet_id, retweeted_by, retweet_tweet_id) VALUES(1,ドル 2,ドル 3ドル) returning id", tweetid, u.Name, retweetid).Scan(&id)
 return fmt.Sprintf("Successfully retweeted tweet by %s", originaluser), retweetid
 }
 }
}
func (u *User) Timeline() []Tweetmodel {
 rows, err := u.Query("select tweets.id, tweets.tweet from tweets INNER JOIN follow ON (tweets.username = follow.following) and follow.username=1ドル", u.Name)
 if err != nil {
 panic(err)
 }
 defer rows.Close()
 tweets := make([]Tweetmodel, 0)
 for rows.Next() {
 var id int
 var msg string
 rows.Scan(&id, &msg)
 tweet := Tweetmodel{id, msg}
 tweets = append(tweets, tweet)
 }
 return tweets
}
func (u User) Tweet(msg string) (string, int) {
 var id int
 u.QueryRow("INSERT INTO tweets(username, tweet) VALUES(1,ドル 2ドル) returning id", u.Name, msg).Scan(&id)
 return "Successfullly tweeted", id
}
func (u *User) Unfollow(usertounfollow User) string {
 res, _ := u.Query("SELECT * from follow where username=1ドル and following=2ドル", u.Name, usertounfollow.Name)
 if res.Next() != true {
 return "You do not follow this user"
 } else {
 u.Exec("DELETE FROM follow WHERE name=1ドル and following=2ドル)", u.Name, usertounfollow.Name)
 return fmt.Sprintf("You have successfully unfollowed %v", usertounfollow.Name)
 }
}
Ethan Bierlein
15.9k4 gold badges59 silver badges146 bronze badges
asked Oct 21, 2015 at 10:20
\$\endgroup\$
3
  • \$\begingroup\$ Why do Login and Tweet operate on a bare user while the other methods use a pointer? I've read the basics but haven't seen this distinction yet. \$\endgroup\$ Commented Oct 21, 2015 at 19:10
  • \$\begingroup\$ @DavidHarkness It was just a matter of preference, nothing more. It is actually better to use pointer methods as there is no copying involved. I was learning Golang at that point of time and went ahead with normal method itself. \$\endgroup\$ Commented Oct 23, 2015 at 8:05
  • \$\begingroup\$ A non-pointer receiver cannot change the orginal object \$\endgroup\$ Commented Apr 21, 2018 at 21:21

1 Answer 1

2
\$\begingroup\$

I am aware this is old but here goes:

  • Can you confirm that *sql.Tx.Query is a proper query builder that prevents sql injection attacks (I don't know).
  • You are correctly closing the rows using defer.
  • Your Follow function could do the down selection on the database using a where clause to determine if you are not already following without requiring two queries (to distinguish between non-existent user and already following, you would have to return a count of the unfiltered username as well).
  • DO NOT store a users password; instead have a random per user salt generated and stored to perform multiple (10 say) becrypt rounds to get a suitably secure hash.
  • You can probably reduce the number of retweet queries by using a slightly more complex version of the following approach
  • Your code implies discrete polling for tweets doing table searches; this gets inefficient quickly with a n * m * u load on your database (n ~ number of people, m ~ number of following per person, u ~ update rate)
    • A much lower processing load (but greater storage) way is to have sharded tweet and followed message tables containing a list of tweets for a users attention (referencing the original tweet entry) so that you only need to do a simple search of a single shard of the followed tweet table for a given user to find all tweets for that users attention without doing table joins. Tweets of interest are pushed onto the sharded followed tweet tables when the original author tweets. The tweet and followed message tables are sharded by the tweeting user and following user respectively.
  • You could have a flag in the sharded user table that is set whenever there are new tweets to display, again avoiding excessive database loading by re-scanning an entire tweet table for new tweets.
  • You don't show how you would be doing app/web page updates; are you fast polling (bad idea), long polling (minimum fallback), a push notification service (which can get costly) or using websockets (good if it works for a given user but fiewalls and proxies often interfere) ?

NB: Reducing the sizes of tables, the number and complexity of queries to a minimum is essential to maintain system performance and minimise cost

answered Apr 21, 2018 at 21:51
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.