Improve Error Message in SimpleDocxProcess when No Document is Present

ingomohr commented 3 years ago

In SimpleDocxProcessor, when you work on a document and you didn't create one before, the error message just points out that there is no doc yet. It doesn't, however, point out how to create one - e.g. via method createDocument().

The task is to improve the error message to give a hint on how to create the document.

Ejaz29 commented 3 years ago

hi @ingomohr I am beginner to opensource but I would like to contribute to the issue if it is open. Can you help me?

ingomohr commented 3 years ago

Hi @Ejaz29! Thank you for getting in touch.

Are you familiar with Java projects? I could also provide you with a small how-to on how to on working on the project - if you think that could help you.

I just re-reviewed the code and I think there actually is less to do than I thought when I created this issue. Well, that could be good for you if you wanted to take the opportunity to have something small to contribute. ;)

What's the problem?

Let's say you have the following code in some Java class: This is the simplest way of using the SimpleDocxProcessor. It will create a new *.docx file containing only "Hello world!"

SimpleDocxProcessor processor = new SimpleDocxProcessor();

//  processor.createDocument();

processor.addMarkdown("Hello **world**!");

try {
    File tempFile = File.createTempFile("docwriter-eclipse-test", ".docx");
    processor.saveDocumentToPath(tempFile);
} catch (IOException e) {
  // log
}

Actually, it won't create that file - and that is because the createDocument() method is not called. A document, however, is required to exist - either by creating one or by loading one.

Executing the snippet above in a (Java class with a main method) will produce a NullPointerException with following message.

"No document found. See createDocument() and loadDocument() methods."

When I created this issue, I missed that I already had added some hint to the doc creation - the "See createDocument()...". But I would like to have both the exception type and the message changed - and here is why:

NullPointerExceptions often don't provide messages - also they typically don't give the impression to be designed to point out a specific problem
- So, maybe they can be misinterpreted easily (which apparently led me to creating this ticket yesterday :) )
The message states the methods - but it doesn't state where they can be found or what to achieve by them.

So what do do?

My suggestion is to

replace the NullPointerException with an IllegalStateException
- This maybe would fit a little better because the processor object in use is actually in a state where it cannot continue to work
replace the error message with something that gives the reader the idea of
- a document can be created and/or loaded by the very same processor object
- and with what methods that can be achieved.
update the corresponding unit test accordingly. It is org.ingomohr.docwriter.docx.TestSimpleDocxProcessor.
optional: I wanted to add some more information incl. some small example of how to use the SimpleDocxProcessor in its class javadoc.

@Ejaz29 I you have some other suggestion or thoughts on the issue you want to share, I would appreciate that. :)

Ejaz29 commented 3 years ago

Thank you for the detailed explanation @ingomohr :) I will work on it. You can also provide me with how to work on a project that would be really helpful. Thank you.

ingomohr commented 3 years ago

https://github.com/ingomohr/docwriter/blob/master/src/test/java/org/ingomohr/docwriter/docx/TestSimpleDocxProcessor.java

@Ejaz29 Are you sure? I see a test class by that name. 😉

The goal for this issue is to update the test to the new expectation (as suggested above) and to update the code so that all tests pass again.

Generally, this is what you can do:

Fork the repo
Make the change
Create a pull request

What knowledge do I need?

Very basic Java understanding
Very basic understanding of Maven
Very basic understanding of JUnit 5

What tools do I need?

a text editor or some IDE
- I use Eclipse and sometimes VSCode
- You can try IntelliJ, too
JDK installed (you can use AdootOpenJDK or OpenJDK), version 11 or higher
Maven installed, latest version

How to Build the Project and run the tests

Clone the repo (i.e. clone your fork)
Go to the root folder of the reoo
Open the terminal or cmd or powershell
Run '‘mvn verify‘‘
- this will both build the project and run the tests

Ejaz29 commented 3 years ago

Sorry @ingomohr I didn't notice. I am good in java but do not have even basic understanding of maven and JUnit5. So if someone else want to contribute they can. I will still be lurking around waiting for the right issue for me to contribute in your repo. :) Thank you for the detailed explanation and I am sorry to waste your time.

ingomohr commented 3 years ago

Hey @Ejaz29, thanks for the update. And you didn't waste my time, don't worry. :) Should you change your mind: I won't have time to resolve this issue myself before tomorrow evening.

ingomohr / docwriter