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.

Sign up for GitHub

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

Jump to bottom

IllegalArgumentException thrown from afterPropertiesSet where IllegalStateException would be more appropriate #2244

Closed
spring-projects-issues opened this issue Jul 14, 2009 · 5 comments
Closed

IllegalArgumentException thrown from afterPropertiesSet where IllegalStateException would be more appropriate #2244

spring-projects-issues opened this issue Jul 14, 2009 · 5 comments
Labels
in: core in: infrastructure in: integration in: samples in: test type: enhancement
Milestone
5.0.0

Comments

@spring-projects-issues
Copy link
Collaborator

Iwein Fuld opened BATCH-1337 and commented

public void afterPropertiesSet() throws Exception {
		Assert.notNull(lineMapper, "LineMapper is required");
	}

Should use Assert.state instead imo.

Affects: 2.0.1
@spring-projects-issues
Copy link
Collaborator Author

Dave Syer commented

Agree. And the problem is fairly ubiquitous in afterPropertiesSet() methods. Maybe the change can be scripted?

@spring-projects-issues
Copy link
Collaborator Author

Robert Kasanicky commented

Maybe we should switch to using @Required on setters?

@spring-projects-issues
Copy link
Collaborator Author

Dave Syer commented

I don't like @Required, and I don't think it's appropriate for framework classes, since it requires the user to add a bean post processor for it to work. InitializingBean is better for my money.

@spring-projects-issues
Copy link
Collaborator Author

Mahmoud Ben Hassine commented

@Iwein Fuld I also agree. However, there are some places in the code base that use Assert.state (like AbstractJob and AbstractStep) while other places use Assert.notNull as you mentioned. It would be great to make this consistent across the whole code base and not only for the FlatFileItemReader. Feel free to open a PR if you want, contributions are welcome!

danilopiazza added a commit to danilopiazza/spring-batch that referenced this issue Feb 7, 2022
@danilopiazza
Throw IllegalStateException from afterPropertiesSet
83bca6d 
Consistently use Assert.state in the afterPropertiesSet() methods to
throw IllegalStateException instead of IllegalArgumentException when
some properties are missing and/or invalid.

Issue spring-projects#2244
@danilopiazza danilopiazza mentioned this issue Feb 7, 2022
Closed
@danilopiazza
Copy link
Contributor

Hi, I opened PR #4055 to address this issue.

@fmbenhassine fmbenhassine added this to the 5.0.0 milestone Apr 8, 2022
@fmbenhassine fmbenhassine removed the status: waiting-for-triage Issues that we did not analyse yet label Apr 8, 2022
@fmbenhassine fmbenhassine linked a pull request Apr 8, 2022 that will close this issue
Throw IllegalStateException from afterPropertiesSet #4055
Closed
@fmbenhassine fmbenhassine changed the title IllegalArgumentException thrown from FlatFileItemReader where IllegalState would be more appropriate [BATCH-1337] IllegalArgumentException thrown from afterPropertiesSet where IllegalStateException would be more appropriate Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
in: core in: infrastructure in: integration in: samples in: test type: enhancement
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

Throw IllegalStateException from afterPropertiesSet danilopiazza/spring-batch
3 participants
@fmbenhassine @danilopiazza @spring-projects-issues