this post was submitted on 02 Mar 2025
8 points (100.0% liked)

Golang

2349 readers
1 users here now

This is a community dedicated to the go programming language.

Useful Links:

Rules:

founded 2 years ago
MODERATORS
 

It appears to pass all tests, but I'm not sure if my test writing skills are necessarily that great, I semi-followed the learn go with tests eBook. I appreciate any feedback, and if this isn't an appropriate post for this community just let me know and I'll remove.

Thanks!!

*** Update *** Updated the repository to be a bit more discriptive of what the library is for and added in a small example project of it in action.

you are viewing a single comment's thread
view the rest of the comments
[–] xtapa@discuss.tchncs.de 2 points 1 week ago* (last edited 1 week ago)

I personally try to avoid deeply nested if/else. Or else in general, because I think it makes code more readable. Especially when one of the branches is just an exit condition.

if exitCondition {
    return false
}
// long ass code execution

is way more readable than

if !exitCondition {
    // long ass code execution
} else {
   return false
}

In a loop, you can just return the value instead of passing it to a "retVal" variable.

With those in mind, you could refactor HasPermissions to

func (r *RBAC) HasPermission(assignedRoles []string, requiredPermission string, visited map[string]bool) bool {
	for _, assigned := range assignedRoles {
		if visited[assigned] {
			continue
		}
		role, ok := r.Roles[assigned]
		if !ok {
			//role does not exist, so skip it
			continue
		}
		for _, permission := range role.Permissions {
			if permission.String() == requiredPermission {
				//Permission has been found! Set permitted to true and bust out of the loop
				return true
			}
		}
		//check inherited roles
		if permitted := r.HasPermission(role.Inherits, requiredPermission, visited); permitted {
			return true
		}
	}
	return false
}

The same could be applied to LoadJSONFile and I think that really would approve the readability and maintainability of your code.

edit: This refactor is not tested