[melbourne-pug] FroSolPy Fronius Inverter Data Collector / Code Feedback

[Alaa Salman]

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-request at python.org wrote:
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Fri, 18 May 2018 09:35:16 +1000
> From: David Crisp <david.crisp at gmail.com>
> To: Melbourne Python Users Group <melbourne-pug at python.org>
> Subject: [melbourne-pug] FroSolPy Fronius Inverter Data Collector /
> 	Code Feedback
> Message-ID:
> 	<CAG3JqCt9nq=aFSxLdeg08NWDLzRttoCC65UNj2USawD2kV1j8A at 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>
>
> ------------------------------
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/melbourne-pug/attachments/20180518/60adce24/attachment-0001.html>