Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2897 Drop out old code related to confirm splitting when printing #2904

Merged
merged 22 commits into from
Nov 8, 2017

Conversation

cristinamghita
Copy link
Member

#2897 Drop out old code related to confirm splitting when printing

@metas-ts metas-ts self-assigned this Nov 7, 2017
@@ -511,6 +501,7 @@ public I_C_Print_Job_Instructions createPrintJobInstructions(final I_C_Print_Job
instructions.setC_PrintJob_Line_From(firstLine);
instructions.setC_PrintJob_Line_To(lastLine);
instructions.setCopies(copies);
instructions.setUserOK(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho this column is oboslete now, isn't it? becausae it was only set be a user (after a particular package was printed) via the monitor feature that we remove in this PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! I'll drop it.

* @return number of created print jobs
*/
int createPrintJobs(IPrintingQueueSource source, IPrintJobMonitor monitor);
int createPrintJobs(IPrintingQueueSource source, int adPInstanceId, int parentAsyncBatchId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIR we already talked about this and meanwhile i thought a bit and consulted @teosarca ..
now we saw that all the method does is to forward them to a printingasyncbatch in the finally block
teo made two suggestions:

  • suggestion 1
    • let's extract those two ints into a POJO called e.g. PrintJobContext, and use that one as parameter
    • and let's also add a default method to this interface which is just `createPrintJobs(IPrintingQueueSource source) and which calls the other one one with an exsitng context.
    • goal: users that have no have no pInstanceId and no parentAsyncBatchId should feel no fud about calling a createPrintJobs method if this API
  • suggestion two: somehow split this method. but with my current understanding, i can't even form an opinion about this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Thx!

batchMonitor.finish();
enqueueForPDFPrinting(source, monitor, pdfPrintingJobInstructions, printJobCount);

final PrintingAsyncBatch printingAsyncBatch = PrintingAsyncBatch.builder()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware of this enqueueForPDFPrinting thing before, but now i'm wondering: what does it do?
looking at the javadoc in IPrintJobBL, i don't see why it should always do something about the PDF printing.

how does it work for users that use physical printers?
feel free to also contact me by voice too, but i think in the end there needs to be something written, either by clearer code or by comments

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rename the method to be more clear.

cristinamghita and others added 14 commits November 7, 2017 14:39
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
#2897 Drop out old code related to confirm splitting when printing
* implement the trivial createPrintJobs method as **default** method
directly in IPrintJobBL
* declare PrintJobcontext not in "private" impl package but in public
IPrintJobBL (the best practice is to consider impl a private package)
* simplify the finally-block in PrintJobBL.createPrintJobs using
@default in the context class
  * also avoid passing a null context instance
* use the context class in 
* rename PrintJobcontext to ContextForAsyncProcessing

Drop out old code related to confirm splitting when printing #2897
@metas-ts metas-ts merged commit f7f7e91 into master Nov 8, 2017
@metas-ts metas-ts deleted the gh2897 branch November 8, 2017 08:30

default int createPrintJobs(@NonNull final IPrintingQueueSource source)
{
return createPrintJobs(source, ContextForAsyncProcessing.builder().build());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls introduce and use some ContextForAsyncProcessing.NONE....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants