Hello Alaa,
First off I must apologise for not responding earlier, gmail insists on
filing all my melbourne-pug emails way over there hidden away from my
normal inbox!
Thank you for looking at the code and providing feedback! This is the sort
of useful information I was hoping to get! Once I have had a chance to
process the information I will comment on why i did things originally.
Regards,
David
On 18 May 2018 at 23:33, Alaa Salman <a...@codedemigod.com> wrote:
> 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> <david.cr...@gmail.com>
> To: Melbourne Python Users Group <melbourne-pug@python.org>
> <melbourne-pug@python.org>
> Subject: [melbourne-pug] FroSolPy Fronius Inverter Data Collector /
> Code Feedback
> Message-ID:
> <CAG3JqCt9nq=afsxldeg08nwdlzrttocc65unj2usawd2kv1...@mail.gmail.com>
> <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>
>
> <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
>
>
_______________________________________________
melbourne-pug mailing list
melbourne-pug@python.org
https://mail.python.org/mailman/listinfo/melbourne-pug
