Declared but not used... really?


(Glenn Hancock) #1

In the following code I declare a var and then use it IF I have access to the database. Otherwise the entire process is pointless. I can’t declare the admin var at the top which is where I like all vars defined, nor can I declare it inside the else condition to get this error to quit pestering me.

I see plenty of people climbing a high horse declaring that go rocks because it forces you to write good code but to me this makes zero sense and I have no idea how to make it quit doing stupid stuff like this. Or maybe I’m being stupid and not seeing something.

I can add

_ = admin 

to make the error go away but seriously, is this an example of clean code?
Please help me understand how to deal with situations like this as they seem to be happening to me regularly.

thanks,

var admin bool                                                                                                                                                                                                                                                                                                                                                                                                                                                                         
                                                                                                                                                                                
if err = db.Ping(); err != nil {                                                                                                                                                                                                           
   logging.LogError( 0, 500, "No Database Connection Exists", w )                                                                                                                                                                          
   return                                                                                                                                                                                                                                  
} else {                                                                                                                                                                                                                                   
  vars := req.Header.Get("userhash")                                                                                                                                                                                                      
  if isAdminUser( vars, w ) {                                                                                                                                                                                                             
     rows, err = querytopmenuall.Query( vars )                                                                                                                                                                                            
     admin = true                                                                                                                                                                                                                         
  } else {                                                                                                                                                                                                                                
     rows, err = querytopmenu.Query( vars )                                                                                                                                                                                               
     admin = false                                                                                                                                                                                                                        
  }

(Windzhu0514) #2

Usually it is like this
If you don’t want this style ,you can change you code logical structure


(Cecil New) #3

Do you plan to actually use the admin variable later in the development of this code? At present, like the message says, you declare it (and you set it), but you don’t actually use it.

In other words, you have created a variable for which there is no need or use. The Go compiler lets you know when you have code that needs to be cleaned up like this.

If you want to use in the future, I’d just comment the three lines for now, and if / when you actually use the admin variable, then uncomment them.


(Glenn Hancock) #4

See, I did have it below but evidently it got removed. Smartass compiler. :slightly_smiling_face:


(Andrew Donelson) #5
// Check for valid database connection, if not exit                                                                                                                                                                              
if err = db.Ping(); err != nil {                                                                                                                                                                                                           
   logging.LogError( 0, 500, "No Database Connection Exists", w )                                                                                                                                                                          
   return                                                                                                                                                                                                                                  
} 

// Database is good, lets get our header data
vars := req.Header.Get("userhash")

// isAdminUser returns a bool already, so admin = isAdminUser()                                                                                                                                                                                                      
if isAdminUser( vars, w ) {                                                                                                                                                                                                             
    rows, err = querytopmenuall.Query( vars )
    // we did the admin query, so we are done here
    return                                                                                                                                                                                            
}                                                                                                                                                                                                                                

// Standard user, do limited query
rows, err = querytopmenu.Query( vars )                                                                                                                                                                                               

So, I am assuming this is a function and not all in your main. With that assumption…The database code should not even be there. You should have already checked for active connection and logged in with the database user information. One time and pass in the reference to the database (db *DB).

I think the easiest way to remember how write go is to just think sequential. You do not need a lot of if/else statements if you use error and return values. For example, I stated that your DB stuff should have already been done, and lets say you want isAdmin accessable You could have had a dbConnect function like so:

  // connection to DB?
  // DBUser logged in?
 // Database / Tables ready?
 // is the user an admin?
// return (dbReference, isAdmin, err)
}

Anyway this straight through approach is simple and works very well.