Dumping html from http request

[gojo (qwerty)]

I have a function for making http requests and storing the response json in an interface. Here I can chain NewDecoder() and Decode() functions to decode and store the json in one line and handle the err value at the caller

func httpRequest(req *http.Request, client *http.Client, r interface{}) error {
resp, err := client.Do(req)
//check error and proceed accordingly
//finally return to the caller
return json.NewDecoder(resp.Body).Decode(&r)
}

Can I do something similar if the I were to return actual html from a webpage using the same function? In this case r would be a []byte type.
I can do this using ioutil.Readall but I don’t want to change the function definition and return just the error after dumping the http response to a variable.

[joncalhoun]

I’m not sure I necessarily agree with the decision to bundle this all into one function the way you are doing it, but if you need that to work you can do it with something like this:

func main() {
	req, _ := http.NewRequest(http.MethodGet, "https://www.google.com/", nil)
	var out []byte
	httpRequest(req, http.DefaultClient, &out)
	fmt.Println(string(out))
}

func httpRequest(req *http.Request, client *http.Client, r interface{}) error {
	resp, err := client.Do(req)
	//check error and proceed accordingly
	//finally return to the caller

	isHtml := true // or false - you can calculate that on your own
	if isHtml {
		br, ok := r.(*[]byte)
		if !ok {
			return errors.New("invalid type provided")
		}
		bytes, err := ioutil.ReadAll(resp.Body)
		if err != nil {
			return err
		}
		*br = bytes
		return nil
	} else {
		return json.NewDecoder(resp.Body).Decode(&r)
	}
}

https://play.golang.org/p/uUkjti5S2eC (Note: You can’t run this on the go playground as the HTTP requests wont be made)

Having said that, would it make more sense to pass in a closure or something similar to this function? From the sound of it you need to know what the return type is going to be before calling httpRequest, so why not try something like this:

func httpRequest2(req *http.Request, client *http.Client, decode func(io.Reader) error) error {
	resp, err := client.Do(req)
	//check error and proceed accordingly
	//finally return to the caller
	return decode(resp.Body)
}

func decodeHtml(dst *[]byte) func(io.Reader) error {
	return func(r io.Reader) error {
		bytes, err := ioutil.ReadAll(r)
		if err != nil {
			return err
		}
		*dst = bytes
		return nil
	}
}

func decodeJson(dst interface{}) func(io.Reader) error {
	return func(r io.Reader) error {
		return json.NewDecoder(r).Decode(dst)
	}
}

Usage is shown in a more complete example here: https://play.golang.org/p/v3yoFd3NGI4 (Note: You can’t run this on the go playground as the HTTP requests wont be made)

There may also be better designs out there for what you are doing, but with my limited knowledge this is about as much guidance as I can offer.

[gojo (qwerty)]

Thank you, I agree that the second option with closure looks much more readable and comparatively simpler than the first.

May I also ask that why is it a bad practice to do it the way I was doing? I mean one thing I noticed right off the bat was that my original function was unecessarily complex with juggling different return types and conditional statements.

[joncalhoun]

Code is always about trade-offs of one sort or another. What you had at first could work, but as you said it was a little complex and was about to start handling two fairly different pieces of logic. Instead I often find it is easier to split that code into two different pieces that can each be tested independently.

I also consider it a code smell when you have a function determining information that something calling it must know ahead of time. In this case it was the format of the data being returned by the http request - any code calling httpRequest must know whether it is JSON, HTML, or some other format in order to pass in the correct r value.

Rather than writing code to determine the Content-Type inside httpRequest (which could be unset, set incorrectly, etc), it seemed simpler to just pass in a decode function that took that into account.

[Giulio_Iotti]

By the way, you don’t have to read it all in memory (it might be big), you can io.Copy to a io.TeeReader like in the example: https://golang.org/pkg/io/#TeeReader