2
\$\begingroup\$

I have a method to check if the Id exists in the data or not, for example:

func (r *ReviewData) Exist() bool {
    // TODO :  checking the shopID value is nil or not.
    // it is a good way to check the value before go to the execution
    query := fmt.Sprintf(
        `
        SELECT
        'x'
        FROM
        ws_active_seller
        WHERE
        shop_id = %d
        `,
        r.ShopID,
    )

    result := postgres.QueryRow(query)
    var data []uint8
    result.Scan(&data)

    if len(data) < 1 {
        return false
    }

    if data[0] != 'x' {
        return false
    }

    return true
}

Notice here that I'm not using any parameters to pass into method here. But There is another approach to do this with :

func (r *ReviewData) Exist(shpoID int64) bool {
    // TODO :  checking the shopID value is nil or not.
    // it is a good way to check the value before go to the execution
    query := fmt.Sprintf(
        `
        SELECT
        'x'
        FROM
        ws_active_seller
        WHERE
        shop_id = %d
        `,
        shopID,
    )
    // the rest is the same 
}

The different with two method above is the parameters. I would like to know what's a good practice for this.

Any ideas on what can be improved?

\$\endgroup\$
1
\$\begingroup\$

In Go you have a choice as to whether you treat structs like an object with its own methods, and you have an object-oriented feel to your code, or you treat data like parameters, and you pass the data around in a more procedural format. Let's look at your first method first.

The ReviewData struct is a struct, and you've declared a method on it that has no parameters. The data required in the method comes from the instance of the struct itself, r.ShopID. The logic there is clear (as an aside .... semantically it's a bit ambiguous because, if you have an r instance, it seems like it should Exist... it's a bit odd to see logic like if abc.Exists() {... because if abc did not exist it seems like abc should be nil ;-)

Regardless, your first version of the code looks very natural to me if you're treating the ReviewData as an "object" with an object-oriented perspective.

The second version of the code is flawed in its current state. The problem is that the second version has the signature:

func (r *ReviewData) Exist(shpoID int64) bool {

but, inside the function, you never use r. Why is it a method on an instance that you never use the instance's data for? It should be just:

func Exist(shopID int64) bool {

(NB: I changed the spelling to be correct... you really should make sure the code works before submitting it to Code Review - broken code is frowned on, and code that does not compile is very frustrating. It could not have compiled with shpoID instead of shopID)

Anyway, the second version is good as a static type of procedure/function, instead of an object instance-based method.

So, depending on how you use the code, the general structure of other methods on ReviewData, and the business logic about whether you create an instance of ReviewData before the shopID exists, or not, determines whether you use the method based version (the first one), or the function version (the second one).

If you store the shopID on the ReviewData instance (and in no other places), then its better to use the first mechanism. If the shop ID is used in multiple places then it may be best to have a package for shops themselves, and then you code could look like if shops.Exists(id) .... (which is the second version).

Bottom line, the right solution depends on how your data is encapsulated, and how you reuse the code.

\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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