DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG?
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://issues.apache.org/bugzilla/show_bug.cgi?id=34828>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND? INSERTED IN THE BUG DATABASE. http://issues.apache.org/bugzilla/show_bug.cgi?id=34828 ------- Additional Comments From [hidden email] 2005-05-10 16:43 ------- **** **** **** **** **** **** **** **** **** **** **** Note: I goofed up a bit on the second **** **** version of the patch #14979 **** **** As a result, you may have to apply the **** **** patch to /src/scratchpad/src instead of **** **** the root folder **** **** **** **** **** **** **** **** **** **** **** Avik, Thanks for the encouraging response 1. Ptg and Eval: Delegating or extending? Initially I had an impl that added hooks into *Ptgs to get/store values. But with input from you and Andy I realised that integrating tightly with Ptgs would mean that the code could not go in until it was completely tested (read a long long time :) IMHO, having Evals separate from Ptg is not too bad since there are only so many *Evals and all the important Evals have been covered. Delegation has the advantage (in this case atleast) of decoupling FormulaParser from FormulaEvaluator. But I guess extending from Ptg could also work... 2. Functions in one class or one class per function? Honestly, I did not give this much thought when I designed - I just assumed one class per function was superior. Thinking about it now, it seems like multiple functions in one class is an alternative - though wrt maintainability i think it would be easier to test-develop-maintain individual classes than one big class. Some of the function implementations themselves can be quite big themselves, so one huge class would be /really/ huge. Also one function per class should make it easier to organize unit tests. Unless one class per function causes problems, I'd vote for one class per function. 3. Testing... Unit testing would be a big effort. Almost as big as writing individual functions, a distributed effort would be great! 4. My eclipse... Yeah, sorry about that. I have two different versions of eclipse, I still havent taught one of them not to do the default comments :) 5. Under contruction: The work on core eval classes is not yet "complete". I think there is need for a BlankEval class to handle empty cells - which are currently being handled as StringEval(""). -- Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
[Moving to the list, since bugzilla is bad for conversation]
Comments inline. > **** As a result, you may have to apply the **** > **** patch to /src/scratchpad/src instead of **** > **** the root folder **** That's ok. For the initial bit, can you just zip/tgz the directory. Provide patches after the initial additions have been comitted. I am waiting to commit till the dummy method javadocs are removed (maybe an awk script might be quick?) > > 1. Ptg and Eval: Delegating or extending? > <snip> But I guess extending from Ptg could also work... Just to clarify, by extend, i meant have a separate class that interits from the Ptgs.. ie, `class AddEval extends AddPtg` .. but as I said, just a suggestion, you'll know best. > 2. Functions in one class or one class per function? <snip> Yeah, sure. > 3. Testing... > Unit testing would be a big effort. Almost as big as writing individual > functions, a distributed effort would be great! Certainly, probably bigger. So once again, this is a call for everybody interested in this functionality to write unit tests for it. Its easy! > > 4. My eclipse... <snip> Please try and remove the extraneous comments, i dont want to put that in CVS. I'll commit it as soon as that's done, I'm happy with everything else. > 5. Under contruction: > The work on core eval classes is not yet "complete". I think there is > need for a > BlankEval class to handle empty cells - which are currently being handled as > StringEval(""). Yeah, sure, thats understood, and no issues. But your 'empty cell' comment made me think, the function implementations should probably follow Excel semantics.. for eg, in the average function, empty cells do not add to the number of elements (ie, the denominator). Also, these semantics might be different for different versions of excel. How do we handle that. A final thoguht on error handling. Your error handling is modelled after OO.o. In excel however, errors are a more limited set.. #REF, #VALUE, etc. Do we need to think about how to handle/map those. Maybe it doesnt matter? Once again, to everybody out there, this is a big and important piece of work that can certainly do with more evaluation/comments/unit tests. Thanks! Regards - Avik --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
-1 on committing any patches without unit tests anymore. It
destabilizes code too much. Even to scratchpad because then the code never moves out of scratchpad. We learned this lesson already. It doesn't matter how good or bad or how much we want it...no unit tests + doco...it stays as patches in bugzilla. [hidden email] wrote: > [Moving to the list, since bugzilla is bad for conversation] > > Comments inline. > >> **** As a result, you may have to apply the **** >> **** patch to /src/scratchpad/src instead of **** >> **** the root folder **** > > > That's ok. For the initial bit, can you just zip/tgz the directory. Provide > patches after the initial additions have been comitted. I am waiting to > commit > till the dummy method javadocs are removed (maybe an awk script might be > quick?) > >> >> 1. Ptg and Eval: Delegating or extending? >> <snip> But I guess extending from Ptg could also work... > > > Just to clarify, by extend, i meant have a separate class that interits > from the > Ptgs.. ie, `class AddEval extends AddPtg` .. but as I said, just a > suggestion, > you'll know best. > >> 2. Functions in one class or one class per function? > > <snip> > > Yeah, sure. > >> 3. Testing... >> Unit testing would be a big effort. Almost as big as writing individual >> functions, a distributed effort would be great! > > > Certainly, probably bigger. So once again, this is a call for everybody > interested in this functionality to write unit tests for it. Its easy! > >> >> 4. My eclipse... > > <snip> > > Please try and remove the extraneous comments, i dont want to put that > in CVS. > I'll commit it as soon as that's done, I'm happy with everything else. > >> 5. Under contruction: >> The work on core eval classes is not yet "complete". I think there is >> need for a >> BlankEval class to handle empty cells - which are currently being >> handled as >> StringEval(""). > > > Yeah, sure, thats understood, and no issues. But your 'empty cell' > comment made > me think, the function implementations should probably follow Excel > semantics.. > for eg, in the average function, empty cells do not add to the number of > elements (ie, the denominator). Also, these semantics might be different > for > different versions of excel. How do we handle that. > > A final thoguht on error handling. Your error handling is modelled after > OO.o. > In excel however, errors are a more limited set.. #REF, #VALUE, etc. Do > we need > to think about how to handle/map those. Maybe it doesnt matter? > > Once again, to everybody out there, this is a big and important piece of > work > that can certainly do with more evaluation/comments/unit tests. Thanks! > > Regards > - > Avik > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > . > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
Am Dienstag, den 10.05.2005, 20:38 -0700 schrieb
[hidden email]: > -1 on committing any patches without unit tests anymore. It > destabilizes code too much. Even to scratchpad because then the code > never moves out of scratchpad. We learned this lesson already. It > doesn't matter how good or bad or how much we want it...no unit tests + > doco...it stays as patches in bugzilla. I second that. Better fewer functionality but tested and documented than vice versa. Best regards Rainer Klute Rainer Klute IT-Consulting GmbH Dipl.-Inform. Rainer Klute E-Mail: [hidden email] K?rner Grund 24 Telefon: +49 172 2324824 D-44143 Dortmund Telefax: +49 231 5349423 Inhibit software patents: http://www.nosoftwarepatents.com/ --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Avik Sengupta
--- [hidden email] wrote:
> -1 on committing any patches without unit tests > anymore. It > destabilizes code too much. Even to scratchpad > because then the code > never moves out of scratchpad. We learned this > lesson already. It > doesn't matter how good or bad or how much we want > it...no unit tests + > doco...it stays as patches in bugzilla. > Wow... Ok. Fine by me. But... * Unit tests: do you mean *all* unit tests, some unit tests that test basic flows or unit tests with complete code coverage? (I'm asking for the min criteria to get this into scratchpad) I hope you mean its ok to have failing unit tests for functionality that is not yet implemented? Also I guess that means that I'm on my own till the code moves into CVS? Again, fine by me, but if there are any ideas to enable distributed development w/o putting it into CVS, they are welcome. * Docos... I assume javadocs. Correct me if you meant otherwise... ~ amol --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by andy-2
On Tue, 2005-05-10 at 20:38 -0700, [hidden email] wrote:
> -1 on committing any patches without unit tests anymore. It > destabilizes code too much. Even to scratchpad because then the code > never moves out of scratchpad. We learned this lesson already. It > doesn't matter how good or bad or how much we want it...no unit tests + > doco...it stays as patches in bugzilla. > While I agree with your general sentiments, I think that not getting things into CVS detracts from getting more contributors, particularly for this formula stuff, where unit tests are going to be easy to write, but we need lots and lots of it, and one person is certainly not going to finish all cases. That's why I think we have scratchpad, so that people can start looking at incomplete stuff. If required, we should have criteria for removing stuff from scratchpad, if they remain unmaintained for too long. Also we should NEVER release jars of stuff from scratchpad, only CVS. If scratchpad and main have the same criteria, then why have scp? would you rather that HWPF not have been added to CVS, as opposed to being where it is now? (sorry, too many rhetorical q's :) Amol, irrespective of this disucussion, you should start writing some tests for your code :) Regards - Avik > [hidden email] wrote: > > [Moving to the list, since bugzilla is bad for conversation] > > > > Comments inline. > > > >> **** As a result, you may have to apply the **** > >> **** patch to /src/scratchpad/src instead of **** > >> **** the root folder **** > > > > > > That's ok. For the initial bit, can you just zip/tgz the directory. Provide > > patches after the initial additions have been comitted. I am waiting to > > commit > > till the dummy method javadocs are removed (maybe an awk script might be > > quick?) > > > >> > >> 1. Ptg and Eval: Delegating or extending? > >> <snip> But I guess extending from Ptg could also work... > > > > > > Just to clarify, by extend, i meant have a separate class that interits > > from the > > Ptgs.. ie, `class AddEval extends AddPtg` .. but as I said, just a > > suggestion, > > you'll know best. > > > >> 2. Functions in one class or one class per function? > > > > <snip> > > > > Yeah, sure. > > > >> 3. Testing... > >> Unit testing would be a big effort. Almost as big as writing individual > >> functions, a distributed effort would be great! > > > > > > Certainly, probably bigger. So once again, this is a call for everybody > > interested in this functionality to write unit tests for it. Its easy! > > > >> > >> 4. My eclipse... > > > > <snip> > > > > Please try and remove the extraneous comments, i dont want to put that > > in CVS. > > I'll commit it as soon as that's done, I'm happy with everything else. > > > >> 5. Under contruction: > >> The work on core eval classes is not yet "complete". I think there is > >> need for a > >> BlankEval class to handle empty cells - which are currently being > >> handled as > >> StringEval(""). > > > > > > Yeah, sure, thats understood, and no issues. But your 'empty cell' > > comment made > > me think, the function implementations should probably follow Excel > > semantics.. > > for eg, in the average function, empty cells do not add to the number of > > elements (ie, the denominator). Also, these semantics might be different > > for > > different versions of excel. How do we handle that. > > > > A final thoguht on error handling. Your error handling is modelled after > > OO.o. > > In excel however, errors are a more limited set.. #REF, #VALUE, etc. Do > > we need > > to think about how to handle/map those. Maybe it doesnt matter? > > > > Once again, to everybody out there, this is a big and important piece of > > work > > that can certainly do with more evaluation/comments/unit tests. Thanks! > > > > Regards > > - > > Avik > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [hidden email] > > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > > . > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Amol Deshmukh-2
Quoting Amol Deshmukh <[hidden email]>:
> > But... > > * Unit tests: > do you mean *all* unit tests, some unit tests that > test > basic flows or unit tests with complete code coverage? not all, some tests, testing the basic functionality. > Also I guess that means that I'm on my own till the > code moves into CVS? Again, fine by me, but if there > are any ideas to enable distributed development w/o > putting it into CVS, they are welcome. > > * Docos... > I assume javadocs. Correct me if you meant > otherwise... javadocs for the primary methods. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Avik Sengupta
Quoting [hidden email]: > Quoting Amol Deshmukh <[hidden email]>: > >> >> * Docos... >> I assume javadocs. Correct me if you meant >> otherwise... > > javadocs for the primary methods. > And a basic how-to .. how to use the api. Even if only a few lines. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Avik Sengupta
TWIC:
I've started writing automated tests and I can see how they are immensely helpful :) So this is my TODO list for now: 1. Write automated tests for the core classes (all OperatorEval impls) 2. Do a few basic functions each with its own set of automated tests as a proof-of concept that functions will work. 3. Get all classes in pkg xxxxx.formula.eval to pass all unit tests. I have written the user api docs and contributors guide which in itself was a GoodThing, since that made me revisit the user api and change it to make it (slightly) more usable. So once I'm done with my TODO list I will submit a patch along with the user and dev docs. Will probably take me another weekend though, so expect (if u care that is :) the next patch around Monday. Hopefully with a couple of more rounds of testing, IMHO, it should be in good shape to be worthy of getting into scratchpad (?) Thanks and Regards, ~ amol > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] > Sent: Wednesday, May 11, 2005 2:11 PM > To: [hidden email] > Subject: Re: FormulaEvaluator Partial Implementation > > > > Quoting [hidden email]: > > > Quoting Amol Deshmukh <[hidden email]>: > > > >> > > >> * Docos... > >> I assume javadocs. Correct me if you meant > >> otherwise... > > > > javadocs for the primary methods. > > > > And a basic how-to .. how to use the api. Even if only a few lines. > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
On Thu, 2005-05-12 at 08:54 -0400, Amol Deshmukh wrote:
> TWIC: > > I've started writing automated tests and > I can see how they are immensely helpful :) > See, told you so! ha ha..! :) <snip> > I have written the user api docs and contributors > guide which in itself was a GoodThing, since that > made me revisit the user api and change it to make > it (slightly) more usable. > Great! have you written them as xdocs? If you want, post them to bugzilla, and I'm happy to convert them to xdocs. You can then maintain them in that format later. > So once I'm done with my TODO list I will submit > a patch along with the user and dev docs. Will > probably take me another weekend though, so expect > (if u care that is :) the next patch around Monday. > I'm sure many people do. > Hopefully with a couple of more rounds of testing, > IMHO, it should be in good shape to be worthy of > getting into scratchpad (?) > I certainly think so. Might be useful to put your wip in bugzilla, if it isnt too much of a pain, I can pitch in if I have some time. --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Avik Sengupta
>
> ------- Additional Comments From [hidden email] 2005-05-12 > 20:38 ------- > Another thought, on the tests... Why do you need separate > test methods/classes > for each eval if you are directly reading it from the excel <snip/> > That way, > one method (with asserts in a loop) will test all > functions/operators. ...because otherwise I wouldnt know which test failed if even one test failed :) Also this way we get more control over which tests we want to carry out simply by specifying the required classes in a testsuite like AllTests.java. Effortwise it is not a big deal since I generated all the classes using code generation (you didnt expect me to /write/ all the test classes did you? :) If there are suggestions to improve after you see the test framework and the test cases, I'll be glad to make necessary changes... Thanks and Regards, ~ amol --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
Some comments inline.
On Thu, 2005-05-12 at 12:30 -0700, Amol Deshmukh wrote: > > > > ------- Additional Comments From [hidden email] > 2005-05-12 > > 20:38 ------- > > Another thought, on the tests... Why do you need > separate > > test methods/classes > > for each eval if you are directly reading it from > the excel > > <snip/> > > > That way, > > one method (with asserts in a loop) will test all > > functions/operators. > > > ...because otherwise I wouldnt know which test failed > if even one test failed :) > would be a separate assert, and in the assert comment, you put the formula. so: while (get all cells) { ... assertEquals(c.getCellFormula(), expectedValueCell, actualValue) } > Also this way we get more control over which tests we > want to carry out simply by specifying the required > classes in a testsuite like AllTests.java. > Well, if you are doing test first development that helps, but really, for regression testing, doesnt matter much, so long as you can see what is failing. And while implementing other functions, if someone wants to do test-first, its better to write a lower level test without reading from an excel file, but doing everything in POI. > Effortwise it is not a big deal since I generated all > the classes using code generation (you didnt expect me > > to /write/ all the test classes did you? :) > No i didnt.... :) but I am thinking less code to maintain! A year later, a new contributor will forget about generation, and copy paste... > If there are suggestions to improve after you see the > test framework and the test cases, I'll be glad to > make > necessary changes... > > Thanks and Regards, > ~ amol > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
In reply to this post by Avik Sengupta
> > > ------- Additional Comments From [hidden email]
> > 2005-05-12 > > > 20:38 ------- > No, you would... you'll get the formula from excel, and each formula > would be a separate assert, and in the assert comment, you put the > formula. so: > > while (get all cells) { > ... > assertEquals(c.getCellFormula(), expectedValueCell, actualValue) > > } But what happens when one assert fails - wouldnt the AssertionError cause the cells that follow to never be tested. (BTW, I'm a JUnit novice, so I may be wrong - in which case we can even continue this discussion off the list to avoid some embarassment to me ;) But I see your point here - I could have a single class & init it for every test case instead of creating new class per function - the issue is maintainability and size of code base. Definitely, like you say my current approach seems to be overkill & not pleasant to maintain... ...I will make changes as per your suggestion in the automated tests. Thanks for your input! ~ amol --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
> But what happens when one assert fails - wouldnt the > AssertionError cause the cells that follow to never be > tested. (BTW, I'm a JUnit novice, so I may be wrong > - in which case we can even continue this discussion > off the list to avoid some embarassment to me ;) > No, you are correct, but my point is when you are doing regression tests, it doesn't matter. The idea is that our regression tests pass 100% for all code in CVS, so if one test fails, you fix that, and then if another fails, you fix that... Obviously, if you are doing test-first, it doesnt work very well, but in that case, you'll be writing different tests in any case. > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > Mailing List: http://jakarta.apache.org/site/mail2.html#poi > The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ > -- --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] Mailing List: http://jakarta.apache.org/site/mail2.html#poi The Apache Jakarta POI Project: http://jakarta.apache.org/poi/ |
Free forum by Nabble | Edit this page |