• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Bear Bibeault
  • Tim Cooke
  • Junilu Lacar
Sheriffs:
  • Paul Clapham
  • Devaka Cooray
  • Knute Snortum
Saloon Keepers:
  • Ron McLeod
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • Frits Walraven
Bartenders:
  • Carey Brown
  • salvin francis
  • Claude Moore

Review on application responsible for fetching data from web txt  RSS feed

 
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys. I would love to hear any opinions from your site about my application.
It is responsible for fetching data from National Polish Bank about buying rate and selling rate from days between given two dates. Next it calculates average buying rate from these days + standard deviation of selling rate.
For example we put 2018-01-28 and 2018-01-31 and it counts 28,29,30,31 of January.

I would like to hear:
1) if tests are properly written.
2) if NBPParserEngine is properly written (what does it mean? - if method names are good, if methods are not too long, if they are, how to shorten them)
3) if DataFetcher is properly written (same as 2) )

To sum up, Im not sure about methods names and if they have proper length.

I made pull request: https://github.com/must1/NBP-Parser/pull/1
If you want to "normal" version of GH: https://github.com/must1/NBP-Parser/tree/dev

I can paste also here some classes:

NBPParserEngine



DataFetcher



Here are tests: https://github.com/must1/NBP-Parser/tree/dev/src/test/java/pl/parser/nbp

Thanks a lot!
 
must Janik
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If needed to paste all classes here, just tell me and I will do it. It's important to me to hear something from "bigger" brains than me :P
 
Sheriff
Posts: 5931
155
Chrome Eclipse IDE Java Postgres Database Ubuntu VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have some minor suggestions.

* You're method and variable names are long, but long is often good.  It's more descriptive.  Here's one place where I might shorten a method name:
Since this method returns a boolean, start the name with "is", maybe something like "isDayIncludedInCurrentYear()"

* I tend to put all my public-facing methods higher in the code and all the private methods lower.  This is so the API is more noticable.

* Sometimes if you have a very long then clause for an if statement...
I instead use a short statement with a return (this is up for debate, though)

* Don't use ALL_CAPS for variable names unless they are constants (static final).
 
must Janik
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks! It means a lot for me.

You're method and variable names are long, but long is often good.  It's more descriptive.  Here's one place where I might shorten a method name:


Done!

* I tend to put all my public-facing methods higher in the code and all the private methods lower.  This is so the API is more noticable.


Done!

* Sometimes if you have a very long then clause for an if statement...




yea, but in this situation I need to act when line is not null, if it is a null then nothing happens

* Don't use ALL_CAPS for variable names unless they are constants (static final).


I think I used it only when variables were constants.

If anybody has some other suggestions. Let me know!
 
Knute Snortum
Sheriff
Posts: 5931
155
Chrome Eclipse IDE Java Postgres Database Ubuntu VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

must Janik wrote:Thanks! It means a lot for me.


You're welcome.

* Sometimes if you have a very long then clause for an if statement...




yea, but in this situation I need to act when line is not null, if it is a null then nothing happens


My suggestion was to put a test for null at the top of the method, then do all the other stuff if it gets past the test.  So to be clearer:


* Don't use ALL_CAPS for variable names unless they are constants (static final).


I think I used it only when variables were constants.


Constants are both static and final, and usually are member variables (not local).
 
must Janik
Ranch Hand
Posts: 82
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

My suggestion was to put a test for null at the top of the method, then do all the other stuff if it gets past the test.  So to be clearer:


Oh, I got it. Done!

Constants are both static and final, and usually are member variables (not local).



Ye, I know it, but the point is I think I used it in application properly. All constants are both static and final.

Greetings!
 
You showed up just in time for the waffles! And this tiny ad:
Create Edit Print & Convert PDF Using Free API with Java
https://coderanch.com/wiki/703735/Create-Convert-PDF-Free-Spire
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!