rgonzalez72/pytrainer — comment on commit 4234291653c4447c5cae56a45c1844f93798eb37
File:  line 0
By rgonzalez72:

Hello Nathan,

thanks for the feedback.

I've been away and I havent looked on the points you make till now.

I was a bit concerned about the naming conventions but I never got to ask.Ill change that.

In your second issue are you suggesting to create a class hierarchy whit a base class called FieldValidator and a number of derived classes for each type (like IntegerFieldValidator or DateFieldValidator)?

I see what you mean in your third point. But, in any case, I think that I need to have of the profile fields related somehow (and the equipment and sport). I mean the fileds cant be tested independently. Imagine they are two wrong fields, after fixing one you should still be able to show the message about the other field.

I dont really understand your suggestion about custom widgets. Can you elaborate a little bit about it?

I'll try to do some code refactoring and make a new proposal.

RODOLFO

________________________________
From: Nathan Jones
To: rgonzalez72
Sent: Monday, 30 January 2012, 9:52
Subject: Re: [pytrainer] Working on issue 81 (4234291)

Hi Rodolfo,

I have three main issues with this code.

Firstly, your naming conventions are not so good. Python variable and method names should all be lower camel case and you should avoid using non-descriptive names. For example, "validateFields" should be "validate_fields" and "retVal" should be "is_valid".

The second issue is use of the "type code" anti-pattern. Instead of passing around type codes and looking up related properties in maps you should just have different objects for each type which contain the properties directly.

The third issue is that you have unsuccessfully tried to make a generic input validation framework but have only made the code more complicated in the process. "FieldValidator" sounds generic but it should actually be called "ProfileFieldValidator" because it has built-in knowledge of those specific fields.

I would expect a better generic approach would be to create custom widgets which implement some validation interface and fire "validation_state_changed" events to indicate when they become valid or invalid.

---
Reply to this email directly or view it on GitHub:
https://github.com/rgonzalez72/pytrainer/commit/4234291653c4447c5cae56a45c1844f93798eb37#commitcomment-913474