Hi David,

I am not sure what kind of code review you're after but I had a quick look at the code and though I can't comment on the functionality, there are a few things that stand out.

1- That's a lot of code to sit in a single file, you might want to consider splitting it up over multiple files or modules

2- The code pattern where you check for key existence before you fetch its value can be replaced with the get() dict function which would cut down on a lot of the code. Some lines in there can also be replaced with a default dict.

3- You'll find that most python code follows the directions in pep8 for style. There's no right or wrong for this one just convention/consistency and tradition.

4- I am not sure why you're doing __new__.__defaults__ = (None,) * len(self.CommonInverterValues.IAC._fields). There are more expressive ways to do this.

5- Something like "url={protocol}://{host}...." can be extracted to a common location since usually this wouldn't change for a single invocation.

6- Using named tuples inside of classes might make your code a bit more difficult to follow. Maybe consider encapsulating the different objects in different classes and populate those.

7- You mentioned there are no tests. I don't see any logic in there that could use testing. However, you can write tests to make sure that your code handles unexpected values as a start.


I hope this helps. Very nice work on commenting your code and keeping it tidy. Please feel free to ask any questions you might have to the list and I'm sure we're all happy to help.



On 18/05/18 14:05, melbourne-pug-requ...@python.org wrote:
----------------------------------------------------------------------

Message: 1
Date: Fri, 18 May 2018 09:35:16 +1000
From: David Crisp <david.cr...@gmail.com>
To: Melbourne Python Users Group <melbourne-pug@python.org>
Subject: [melbourne-pug] FroSolPy Fronius Inverter Data Collector /
        Code Feedback
Message-ID:
        <CAG3JqCt9nq=afsxldeg08nwdlzrttocc65unj2usawd2kv1...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"

Gday,

I'm not sure if this is appropriate to ask for or not but I was wondering
if there was anybody who would be happy to do a quick code review / code
feedback on my Fronius Solar module I have written  and give me some
feedback on it.

I have been working on this module for a while and I think I'm beginning to
not be able to see the trees for the forest.   It is NOT finished yet but
it does what I need it to do for the moment.

There's no unit tests though.  I haven't worked out how to do these for
dynamic data collected from APIs etc which could return anything.

Currently being unemployed and not having access to a development team I
don't get a chance to drop code in front of more experienced people and get
ideas from them.

The module should be able to be found at the following location.
https://github.com/dcrispgit/FroSolPy

Bonus Points if you have your own Fronius solar inverter and you can
actually run this code and retrieve data from it.

If it's not appropriate to ask that then feel free to ignore or point me in
the direction of somewhere that can help.

Regards,
David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://mail.python.org/pipermail/melbourne-pug/attachments/20180518/8219323b/attachment-0001.html>

------------------------------

_______________________________________________
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug

Reply via email to