Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is there a more efficient approach to this code?

// save associated tag
let saveTag question = 

    if question.Tag = null then
        ()
        else
            let tagCount = 
                query{
                    for row in db.Tags_Tags_Tags do
                    where (row.Tag = question.Tag)
                    select row
                    count
                } 

            if tagCount > 0 then
                ()
                else
                    let tagToSave = 
                        dbSchema.ServiceTypes.Tags_Tags_Tags(
                            Tag = question.Tag
                        )
                    insertRowIn db.Tags_Tags_Tags tagToSave
                    saveToDb()
share|improve this question
add comment

2 Answers 2

up vote 4 down vote accepted

You don't need the empty if branches. An if without an else automatically returns unit. So you can simplify it to:

let saveTag question = 

    if question.Tag <> null then
        let tagCount = 
            query{
                for row in db.Tags_Tags_Tags do
                where (row.Tag = question.Tag)
                select row
                count
            } 

        if tagCount = 0 then
            let tagToSave = 
                dbSchema.ServiceTypes.Tags_Tags_Tags(
                    Tag = question.Tag
                )
            insertRowIn db.Tags_Tags_Tags tagToSave
            saveToDb()
share|improve this answer
add comment

In addition to Daniel's answer, the select row in your tagCount query is redundant. You can simplify the query to:

    let tagCount = 
        query {
            for row in db.Tags_Tags_Tags do
            where (row.Tag = question.Tag)
            count
        } 

But you could further improve your query. In this case you don't actually need to know the exact count, you just need to know if there's at least one match or not.

let tagExists =
    query {
        for row in db.Tags_Tags_Tags do
        exists (row.Tag = question.Tag)
    }

if not tagExists then
    ...

This way the query can stop processing as soon as it finds a match, instead of continuing on until it has an exact count of how many matches. It's a bit more efficient that way when there's lots of data.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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