-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamically load pages based on string parameter #58
Comments
You can try to put all the var templates = map[string]func(w io.Writer, data authboss.HTMLData) {
"login_page": templates.WriteLoginPage,
"main_page": templates.WriteMainPage,
} All the
Then the map could be used from var bb bytes.Buffer
f := templates[page]
f(&bb, data)
return bb.Bytes(), "text/html", nil Probably this would work, but personally I don't like this abstraction, since it adds a level of indirection - a map of template functions - which makes code less clear. Additionally it restricts template functions to accept only a single argument - |
What a great honor for me to read your answer, @valyala. Thank you so much. I thought of a similar solution ( And I don't like it as a solution too. Could you explain me better what you mean by:
I think it's the only way to use |
What do you think about the code below, @valyala?
type AuthPage interface {
Title() string
}
type LoginPage struct {
Data authboss.HTMLData
}
func (lp *LoginPage) Title() string {
return "title"
}
func InitializeLoginPage(data authboss.HTMLData) AuthPage {
return &LoginPage{Data: data}
}
type RecoverPage struct {
Data authboss.HTMLData
}
func (rp *RecoverPage) Title() string {
return "title"
}
func InitializeRecoverPage(data authboss.HTMLData) AuthPage {
return &RecoverPage{Data: data}
}
func main() {
templates := map[string]func(authboss.HTMLData) AuthPage{
"login": InitializeLoginPage,
"recover": InitializeRecoverPage,
}
newLoginPage := templates["login"](data)
newRecoverPage := templates["recover"](data)
} |
I have no experience with |
Ok. Thanks. Can you tell me about the code in previous comment (#58 (comment))? After I can close this issue. I tried with factory pattern. What do you think? |
I think it is better to use plain old // WritePage write page for the given templateName and the given arg into w.
func WritePage(w io.Writer, templateName string, args interface{}) {
switch templateName {
case "login":
return WriteLoginPage(w, args)
case "recover":
return WriteRecoverPage(w, args)
default:
fmt.Fprintf(w, "unknown templateName=%q", templateName)
}
} This switch is simpler than the map or factory method because it avoids a level of indirection or two (in case of factory method), so the code is easier to understand, update and maintain. |
The problemYes, the problem is I need initializers because I'm using a single func:
and every
so I don't have func like What have I doneNow I'm using type HTML struct {
...
templates map[string]func(authboss.HTMLData) templates.PageImpl
...
}
func InitializeLoginPageType(data authboss.HTMLData) templates.PageImpl {
return &templates.LoginPage{Data: data}
}
func (h *HTML) Load(names ...string) error {
for _, n := range names {
switch n {
case "login":
h.templates[n] = InitializeLoginPageType
}
}
return nil
}
func (h *HTML) Render(ctx context.Context, page string, data authboss.HTMLData) (output []byte, contentType string, err error) {
buf := &bytes.Buffer{}
tpl, ok := h.templates[page]
if !ok {
return nil, "", errors.Errorf("...")
}
templates.WritePage(buf, tpl(data))
return buf.Bytes(), "text/html", nil
} The million dollar questionDo you see problems with this code? Do you see parts to improve? |
This approach looks OK from the first sight. |
Your advice is invaluable, thank you very much. I am still learning Go and every problem is an opportunity to learn, but only if I am "guided" by skilled and generous people like you. I have created two different versions of the solution and would like to know which one is best or if there is a third one better.
I think you prefer the latter, but I don't know what to improve for performances.
|
Both versions look almost identical. Take the simplest version which will be easier to understand and maintain in the future. As for the performance, both versions contain superfluous memory allocation when returning the byte slice from |
I can ask author to get rid of it. Can I ask you what are you using for authentication when you cannot use third-party services (e.g. auth0)? |
I usually abstract away the authentication into a separate package if err := auth.Authenticate(req); err != nil {
return fmt.Errorf("authentication failure on request %s: %s", req, err)
} |
Ok. However, I mean the technology that you usually use as an alternative to everything that Something like But I think I'm going off-topic. Just your quick answer out of curiosity and then I close. |
First of all thanks a lot for
quicktemplate
, I'm newbie and this is great: I'm learning a lot from your code. Thanks again!(Maybe) a silly question: I'm trying to use
authboss
withquicktemplate
but I don't know if I'm doing it well.Authboss has just one interface (https://godoc.org/github.com/volatiletech/authboss/#Renderer) for its rendering system.
As stated in its README (https://github.com/volatiletech/authboss/blob/master/README.md#rendering-views):
If you read the code for
html.go
you can seeLoad()
andRender()
methods.I started copying that code and if I understand correctly:
Load()
method: I think I don't need it (for my basic work). Am I wrong? The original authboss one is here with some comments of mine:Render()
method I can use something like this:which has the problem I cannot dynamically load pages in
templates.WritePage()
method based onpage
parameter.LoginPage
is coming from a template like this:Maybe with reflection? Really? I read everywhere reflection is really slow and I need to use something else if possible.
I tried also with something like this:
but I think something is wrong here. I don't like
ALL_TEMPLATES
slice way of doing this.What do you suggest?
I can write a Wiki (for newbies like me) in this project and in the authboss one.
I already opened an issue on
authboss
repo: volatiletech/authboss#239.The text was updated successfully, but these errors were encountered: