Granny's Programming Pearls
"inside of every large program is a small program struggling to get out"
JavaRanch.com/granny.jsp
  • 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
  • Devaka Cooray
  • Knute Snortum
  • Paul Clapham
  • Tim Cooke
Sheriffs:
  • Liutauras Vilda
  • Jeanne Boyarsky
  • Bear Bibeault
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Ron McLeod
  • Piet Souris
  • Frits Walraven
Bartenders:
  • Ganesh Patekar
  • Tim Holloway
  • salvin francis

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
 
Marshal
Posts: 5993
156
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
Marshal
Posts: 5993
156
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!
 
How do they get the deer to cross at the signs? Or to read this tiny ad?
how do I do my own kindle-like thing - without amazon
https://coderanch.com/t/711421/engineering/kindle-amazon
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!