Am I missing a design pattern? My views code somehow isn't as elegant as it should be...

[Andrew Chiw]

In my views, I have this:

def questionnaire(request):
    def save_the_lead(cleaned_data, ipinfo):
        email = cleaned_data.pop('email', None)
        lead, created = Lead.objects.update_or_create(email=email)
        lead.q = cleaned_data
        lead.ipinfo = ipinfo
        lead.save()
        return lead

    form = LeadForm(request.POST or request.GET or None)
    """
    Why am I still checking for request.method == "POST"?
    if POST (valid data) to questionnaire, process it.
    if GET (valid data) to questionnaire, display it first. Even if it's all valid.
    """
    if request.method == "POST":
        if form.is_valid():
            i = IPInfo(get_ip(request), dummy=True)
            lead = save_the_lead(form.cleaned_data, i.get_result())
            m = Matcher()
            m.match([lead])
            e = Emailer(lead)
            e.send()
            return redirect('results', lead.id)

    return render(request, 'wizard/questionnaire.html', {'form': form})

And IPInfo looks like this:

import requests
import ipdb


class IPInfo:
    def __init__(self, ip, dummy=False):
        self.result = dict()
        if (ip and not dummy):
            response = requests.get('http://ipinfo.io/' + ip)
            self.result = response.json()
        elif (ip and dummy):
            self.result = {"ip": ip}

    def get_result(self):
        return self.result

The part I'm worried about is the IPInfo lookup class and how it is used in the view. I know you can mock classes in tests, but maybe I should use dependency injection instead? or is that not needed in Python?
If I'm missing dependency injection: I don't have control over the code that runs before the view, so I cannot tell it to inject another instance of IPInfo in there.

[Alex Heyden]

Are you intentionally sending the whole request into that constructor and concatenating it to a URL string? I can't imagine any way this doesn't end in disaster.

I'm sure the community would happily help, but it's not at all obvious from the supplied code what you're trying to do here.

--
You received this message because you are subscribed to the Google Groups "Django users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-users+unsubscribe@googlegroups.com.
To post to this group, send email to django...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-users.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/2777f9b7-ec87-4dc5-ac8f-de971860c83b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Andrew Chiw]

As someone POSTs a questionnaire, I lookup some additional info about the IP and save that into a model's JSONField.
I'm sending the request into django-ipware's get_ip(), which will return the IP address as a string. Since this data is ephemeral, I put it directly in the constructor instead of storing the return value in a variable.
My bad, should've included the imports at the top of the file so you know where get_ip() comes from.

[ludovic coues]

Class Based View really shine in form handling. You can find the
documentation at [1]. All the boilerplate code will be 3 assignment
like template_name = 'wizard/questionnaire.html' while the logic will
go into form_valid function.

I would also add a comment for the Matcher object and why the m
variable isn't used after the call to match.

[1] https://docs.djangoproject.com/en/1.10/topics/class-based-views/generic-editing/

> --
> You received this message because you are subscribed to the Google Groups
> "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to django-users...@googlegroups.com.

> To post to this group, send email to django...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-users/ad4ef983-3430-4058-ae24-e036cd6205f1%40googlegroups.com.

>
> For more options, visit https://groups.google.com/d/optout.

--

Cordialement, Coues Ludovic
+336 148 743 42

[Vinicius Assef]

I'd say you're missing a service layer.

It's known as a good practice to not have "business rules" in your views.

IP lookup should be done in your view anyway, because it's a "web
related thing". The `save_the_lead()` and email sending should be in
the service layer.

Extending a little bit and talking about pythonic code,
`IPInfo.get_result()` should not exist. Simply use the `result`
attribute or convert it to a read only property. Or, if this class
came to life just because the `get_result()` method, convert it to a
simple function.

> --
> You received this message because you are subscribed to the Google Groups
> "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-users...@googlegroups.com.
> To post to this group, send email to django...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-users/2777f9b7-ec87-4dc5-ac8f-de971860c83b%40googlegroups.com.

[Andrew]

Tried to look up service layers, but couldn't find examples of how it was supposed to look like - is it supposed to be like a class, and I toss the lead into it and it handles business logic and saving?

Normally I'd use ipinfo.result myself, but I read somewhere about separating the interface from the internals of the class, so I don't have classes everywhere depending on this variable having this name in IPInfo.

Or perhaps that doesn't apply in Python?

I'm probably really confused...

> email to django-users+unsubscribe@googlegroups.com.

> To post to this group, send email to django...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-users/2777f9b7-ec87-4dc5-ac8f-de971860c83b%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to a topic in the Google Groups "Django users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-users/H_N9ierYHJk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-users+unsubscribe@googlegroups.com.

To post to this group, send email to django...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-users.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/CAFmXjSDVUwCE6Tpaiv_HMRvejkrA6XvSu0zWP0tjfSsUsctHZg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

randomshinichi

....pi......pi......kaaaaaaa....
AMD Thuban 3722MHz, 12288MB DDR3-1466 CL9 SDRAM, Asus M5A99X EVO, nVidia GTX470@840/3800MHz+Gelid Icy Vision Rev. 2

[ludovic coues]

Python doesn't have special structure to separate internal and
interface. There is a convention of prefixing the internal with an
underscore. That's all.

So having IPInfo.request being part of your interface is totally fine.
Or you could keep things as they are. Or rename IPInfo.request to
IPInfo._request and IPInfo.get_request() to IPInfo.request().

>> > email to django-users...@googlegroups.com.

>> > To post to this group, send email to django...@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/django-users.
>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/d/msgid/django-users/2777f9b7-ec87-4dc5-ac8f-de971860c83b%40googlegroups.com.
>> > For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to a topic in the
>> Google Groups "Django users" group.
>> To unsubscribe from this topic, visit
>> https://groups.google.com/d/topic/django-users/H_N9ierYHJk/unsubscribe.
>> To unsubscribe from this group and all its topics, send an email to

>> django-users...@googlegroups.com.

>> To post to this group, send email to django...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/django-users.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-users/CAFmXjSDVUwCE6Tpaiv_HMRvejkrA6XvSu0zWP0tjfSsUsctHZg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --
>
> randomshinichi
>
> ....pi......pi......kaaaaaaa....
> AMD Thuban 3722MHz, 12288MB DDR3-1466 CL9 SDRAM, Asus M5A99X EVO, nVidia
> GTX470@840/3800MHz+Gelid Icy Vision Rev. 2
>

> --
> You received this message because you are subscribed to the Google Groups
> "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to django-users...@googlegroups.com.

> To post to this group, send email to django...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-users/CAEam--U9vTBE1xsvVxMHXPjx_z%2B69bJcUBXFjNEeF7iLD%2B3Xcg%40mail.gmail.com.

>
> For more options, visit https://groups.google.com/d/optout.



--

[Vinicius Assef]

Inline answers follow...

On 23 October 2016 at 10:01, Andrew <randomshi...@gmail.com> wrote:
> Tried to look up service layers, but couldn't find examples of how it was
> supposed to look like - is it supposed to be like a class, and I toss the
> lead into it and it handles business logic and saving?

Keep in mind we're talking about Python, OK? Then, you should not
create a class if you don't need it. Simple functions work nice many
times.

A service layer is just another layer. Your view and template is on
the "web" layer. A service layer would be on the "business" layer,
between your user interface and your domain model. No secret in it.
Just another class or function in another package or module.

See this overview for clarification:
http://martinfowler.com/eaaCatalog/serviceLayer.html

There's no "the right way" to do it. No silver bullet. Design varies
from person to person, but I strongly advocate for thin views
according to the Single Responsibility Principle, i.e, letting views
gather (call) workers (the service layer) to do the "dirty work" and
return a response to user. Nothing more.

>
> Normally I'd use ipinfo.result myself, but I read somewhere about separating
> the interface from the internals of the class, so I don't have classes
> everywhere depending on this variable having this name in IPInfo.

Interfaces are a conceptual stuff. Python doesn't have a word or
structure for them. No additional complexity.

You can emulate interfaces creating a class implementing public
methods (names don't start with an underscore) and simply raising
NotImplementedError. It's children should implement these real
methods.

Some example (Python 3):

class MyInterface:
def do_something(self):
raise NotImplementedError("Must be implemented in child class")

class MyClass(MyInterface):
def __init__(self):
# do some initialisation

def do_something(self):
# real implementation

def _some_private_method(self):
# do something

You see good examples of "programming to an interface" concept when
you read about magic methods in Python docs. These "interfaces" are
expected to make your own comparisons, evaluating True/False or object
length and so on. See docs here:
https://docs.python.org/3/reference/datamodel.html#special-method-names

--
Cheers.
Vinicius Assef

>> > email to django-users...@googlegroups.com.

>> > To post to this group, send email to django...@googlegroups.com.
>> > Visit this group at https://groups.google.com/group/django-users.
>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/d/msgid/django-users/2777f9b7-ec87-4dc5-ac8f-de971860c83b%40googlegroups.com.
>> > For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to a topic in the
>> Google Groups "Django users" group.
>> To unsubscribe from this topic, visit
>> https://groups.google.com/d/topic/django-users/H_N9ierYHJk/unsubscribe.
>> To unsubscribe from this group and all its topics, send an email to

>> django-users...@googlegroups.com.

>> To post to this group, send email to django...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/django-users.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-users/CAFmXjSDVUwCE6Tpaiv_HMRvejkrA6XvSu0zWP0tjfSsUsctHZg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --
>
> randomshinichi
>
> ....pi......pi......kaaaaaaa....
> AMD Thuban 3722MHz, 12288MB DDR3-1466 CL9 SDRAM, Asus M5A99X EVO, nVidia
> GTX470@840/3800MHz+Gelid Icy Vision Rev. 2
>

> --
> You received this message because you are subscribed to the Google Groups
> "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to django-users...@googlegroups.com.

> To post to this group, send email to django...@googlegroups.com.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-users/CAEam--U9vTBE1xsvVxMHXPjx_z%2B69bJcUBXFjNEeF7iLD%2B3Xcg%40mail.gmail.com.