[Context] : I'm currently developing a program in nuclear physics and i would like to have a simple access to all nuclides.
Technical specifications are :
- Since informations are static, I want them to be hard coded. (means no external file to read) -> Enum seems to be a good starting point then.
- Each nuclide should carry :
- Atomic number A
- Mass number Z
- Isomeric number I
- Half decay time
- Natural decay mode
- The access via an API should be really really simple something like
Nuclides.get(Z,A,I)
/Nuclide.get("C14")
or equivalent is recommended. The number of nuclides is almost 3000.
[Remarks] This post is a follow-up question : Initial post
@Antot : Here is the new post.
[Design choices]
According to Antot suggestions I significantly improved the main design of the class. Especially fields/determineAtomicNumber/determineIsomericState :
/**
* This class represents a Nuclide
* <p>
* A nuclide is completely defined by :
* <ul>
* <li>atomic number</li>
* <li>mass number</li>
* <li>isomeric number/state</li>
* </ul>
*
* @author Johann MARTINET
*/
public class Nuclide {
/**All nuclide Symbols (Z order)*/
public static final String[] SYMBOLS = {"H", "He", "Li", "Be", "B", "C", "N", "O", "F", "Ne", "Na", "Mg", "Al", "Si", "P", "S", "Cl", "Ar", "K", "Ca", "Sc", "Ti", "V", "Cr", "Mn", "Fe", "Co", "Ni", "Cu", "Zn", "Ga", "Ge", "As", "Se", "Br", "Kr", "Rb", "Sr", "Y", "Zr", "Nb", "Mo", "Tc", "Ru", "Rh", "Pd", "Ag", "Cd", "In", "Sn", "Sb", "Te", "I", "Xe", "Cs", "Ba", "La", "Ce", "Pr", "Nd", "Pm", "Sm", "Eu", "Gd", "Tb", "Dy", "Ho", "Er", "Tm", "Yb", "Lu", "Hf", "Ta", "W", "Re", "Os", "Ir", "Pt", "Au", "Hg", "Tl", "Pb", "Bi", "Po", "At", "Rn", "Fr", "Ra", "Ac", "Th", "Pa", "U", "Np", "Pu", "Am", "Cm", "Bk", "Cf", "Es", "Fm", "Md", "No", "Lr", "Rf", "Ha", "Sg", "Ns", "Hs", "Mt", "Ds","Rg"};
/**
* All possible isomeric state :
* gs = ground state (I=0)
* m (I=1)
* m2 (I=2)
* m3 (I=3)
*/
public static final String[] STATES = {"gs","m","m2","m3"};
/** Symbol of this nuclide*/
private final String symbol;
/** Atomic number*/
private final int atomicNumber;
/** Mass number*/
private final int massNumber;
/** Isomeric state*/
private final int isomericState;
/** Reactions of this nuclide*/
private final String reactions;
/** Half decay time*/
private final double decayTime;
/**
* Default constructor
* @param symbol symbol of the nuclide
* @param massNumber massNumber of the nuclide
* @param isomericState isomeric state of the nuclide
* @param decayTime half decay time of the nuclide
* @param reactions reactions of the nuclide
*/
private Nuclide(String symbol, int massNumber, String isomericState, double decayTime, String reactions) {
this.symbol = symbol;
this.massNumber = massNumber;
this.atomicNumber = determineAtomicNumber(symbol);
this.isomericState = determineIsomericState(isomericState);
this.decayTime = decayTime;
this.reactions = reactions;
}
/**
* Determine the atomic number from conventional symbol
* @param symbol symbol (should be one of Nuclide.SYMBOLS)
* @return (int) atomic number associated to the given symbol
* @throws IllegalArgumentException if symbol is not included in Nuclide.SYMBOLS
*/
private static int determineAtomicNumber(String symbol) {
for (int i = 0; i < SYMBOLS.length; i++)
{
if (symbol.equals(SYMBOLS[i])){return i + 1;}
}
throw new IllegalArgumentException("Failed to determine atomic number, invalid symbol: " + symbol+ " , should be one of Nuclide.SYMBOLS");
}
/**
* Determine the isomericState from conventional symbol defined in Nuclide.STATES
* @param isomericState String representation of the isomericState (should be one of Nuclide.STATES)
* @return (int) isomeric number associated to the given symbol
* @throws IllegalArgumentException if symbol is not included in Nuclide.STATES
*/
private static int determineIsomericState(String isomericState) {
for(int i = 0; i < STATES.length; i++)
{
if(isomericState.equals(STATES[i])){return i + 1;}
}
throw new IllegalArgumentException("Failed to determine isomeric number, invalid symbol: " + isomericState+ " , should be one of Nuclide.STATES");
}
//...
The question that comes first is :
Is it worth replacing SYMBOLS and STATES by enums ?
/** Isomeric states*/
public enum State
{
/**Ground state*/
GROUND_STATE(0),
/** first isomeric state*/
M(1),
/** second isomeric state*/
M2(2),
/** third isomeric state*/
M3(3);
/** Isomeric number of this state*/
private final int isomericNumber;
/**
* Default constructor
* @param isomericNumber isomericNumber of the state
*/
private State(int isomericNumber){this.isomericNumber = isomericNumber;}
}
public enum Symbol
{
H("H"),
He("He"),
\\...
private final String symbol;
private Symbol(String symbol){this.symbol = symbol)
}
And then change constructor and all nuclides enum like this :
private Nuclide(Symbol symbol, int massNumber, State isomericState, double decayTime, String reactions)
/** Americium*/
public enum Am implements NuclideAware{
\\...
Am242(242, State.GROUND_STATE, 5.767200e+04, "b-:8.270000e+01,ce:1.730000e+01"),
Am242m(242, State.M, 4.449620e+09, "it:9.955000e+01,a:4.500000e-01,fs:1.600000e-08"),
Am242m2(242, State.M2, 1.400000e-02, "fs:1.000000e+02"),
\\...
private Nuclide nuclide;
Am(int A, String isomericState, double decayTime, String reactions) {this.nuclide = new Nuclide(Symbol.Am, A, isomericState, decayTime, reactions);}
public Nuclide getNuclide() {return nuclide;}
}
This way, we can remove determineIsomericState
and determineAtomicNumber
methods and IllegalArgumentException
are not needed anymore.
[Nuclide.get(String ...)]
Once more I used Antot suggestion and I significantly improved this method via OOP and Regex :
/**
* Interface used as a badge for nuclide
* @author Johann MARTINET
*/
public interface NuclideAware
{
/**
* Return the nuclide
* @return nuclide
*/
Nuclide getNuclide();
}
Which gives a more straightforward get method :
/**
* Return a nuclide from the given name. The name should follow this format :
* [Sy][A]m[I]
* where :
* Sy is a symbol included in Nuclide.SYMBOLS
* A is the mass number of the nuclide
* Is is an isomeric state included in Nuclide.STATES
* Example : "Am242m2"
* @param name name of the nuclide (format = Symbol-A-IsomericState)
* @return (Nuclide) The corresponding nuclide
*/
public static Nuclide get(String name)
{
String symbol ="", massNumber ="", isomericState ="";
String regex = "^([A-Za-z]+)(\\d+)m(\\d+)$";
Pattern pattern = Pattern.compile(regex);
Matcher matcher = pattern.matcher(name);
if(matcher.find() && matcher.hitEnd())
{
symbol = matcher.group(1);
massNumber = matcher.group(2);
isomericState = matcher.group(3);
}
final String targetNuclide = symbol + massNumber + "m" + isomericState;
try
{
Class<?> c = Class.forName(Nuclide.class.getName() + "$" + symbol);
Object[] objects = c.getEnumConstants();
for(Object obj : objects)
{
if(obj.toString().equals(targetNuclide))
{
return ((NuclideAware) obj).getNuclide();
}
}
}catch(ClassNotFoundException e)
{
e.printStackTrace();
}
throw new IllegalArgumentException("Impossible to find the nuclide with name:" + name);
}
As Antot already stated in initial post :
This still remains rather brutal and rigid
Then the second question is :
Is there a much more flexible way to implement the entire thing using more OOP features ?
Thank you for reading
2 Answers 2
First, some remarks about the usage of enum
s.
In State
:
GROUND_STATE(0), M(1), ...
The values associated with the elements are no more than their ordinal numbers, so exactly the same effect would be reached if it were defined as
public enum IsomericState {
GROUND_STATE,
M,
M2,
M3;
public int getIsomericNumber() {
return this.ordinal();
}
}
In Symbol
:
H("H"), He("He")
The strings redundantly repeat the literals. He.name()
or He.toString()
will produce the same, so there is no need to add symbol
field.
Now, after all the refactoring applied since the initial question, let's try to approach the design a bit differently, with the aim of simplifying the thing.
In the reviewed solution there is an array to store symbols as strings (Nuclide.SYMBOLS
) and enums per each chemical element (H
, He
...), each wrapping data for nuclides. However, the core notion of this system is the chemical element, with a few constant values for different nuclides (symbol, atomic number) and with other fields varying. So why not creating a single enum
that holds constant properties for each chemical element, used as prototype to initialize a Nuclide
for any of them? This could look like follows:
public enum PeriodicElement {
H,
He,
Li,
// other elements
;
private final int atomicNumber;
PeriodicElement() {
this.atomicNumber = ordinal() + 1;
}
public int getAtomicNumber() {
return atomicNumber;
}
public Nuclide asNuclide(int massNumber,
IsomericState isomericState,
String reactions,
double decayTime) {
return new Nuclide(this, massNumber, isomericState, reactions, decayTime);
}
}
This requires changes in our Nuclide
definition:
package codereview.nuclides;
public class Nuclide {
private final PeriodicElement element;
private final int massNumber;
private final IsomericState isomericState;
private final String reactions;
private final double decayTime;
Nuclide(PeriodicElement element,
int massNumber,
IsomericState isomericState,
String reactions,
double decayTime) {
this.element = element;
this.massNumber = massNumber;
this.isomericState = isomericState;
this.reactions = reactions;
this.decayTime = decayTime;
}
// + getters
@Override
public String toString() {
return element.name() + massNumber + "m" + isomericState.getIsomericNumber();
}
}
We link each
Nuclide
to its parentPeriodicElement
. There is no more risk of having an invalid value for formerly knownsymbol
.The constructor has default visibility. This is done intentionally in order to have complete control where all our Nuclides are instantiated.
Now, the main part of the thing. We need an object that will hold all the Nuclides
that we know and provide access to them. Let's call it NuclidesRegistry
:
package codereview.nuclides;
import java.util.HashMap;
import java.util.Map;
public class NuclidesRegistry {
private static Map<String, Nuclide> REGISTRY = new HashMap<>();
static {
initNuclidesRegistry();
}
private static void registerNuclide(Nuclide nuclide) {
REGISTRY.put(nuclide.toString(), nuclide);
}
private static void initNuclidesRegistry() {
// this part remains ugly...
registerNuclide(PeriodicElement.H.asNuclide(1, IsomericState.GROUND_STATE, "s", 0.000000e+00));
registerNuclide(PeriodicElement.H.asNuclide(2, IsomericState.GROUND_STATE, "s", 0.000000e+00));
registerNuclide(PeriodicElement.H.asNuclide(3, IsomericState.GROUND_STATE, "b-:1.000000e+02", 3.891050e+08));
registerNuclide(PeriodicElement.H.asNuclide(4, IsomericState.GROUND_STATE, "n:1.000000e+02", 1.000000e-22));
// ...
}
public static Nuclide get(String name) {
if (REGISTRY.containsKey(name)) {
return REGISTRY.get(name);
}
throw new IllegalArgumentException("Invalid Nuclide name: " + name);
}
}
The get(String)
method (the former Nuclide.get(String)
) does not use reflection any more and it is reduced to three lines!
The ugly part is still the hard-coded data in initNuclidesRegistry()
. The lines might be shortened a bit with static imports, but are still quite annoying:
import static codereview.nuclides.IsomericState.*;
import static codereview.nuclides.PeriodicElement.*;
...
registerNuclide(H.asNuclide(1, GROUND_STATE, "s", 0.000000e+00));
registerNuclide(H.asNuclide(2, GROUND_STATE, "s", 0.000000e+00));
Since there are about 3000 nuclides and if you cannot use a resource file (a CSV would be ok...), I'd suggest to split this instantiation in groups per elements. But, seriously, an external resource is much better.
-
\$\begingroup\$ Once more thank you for this detailed answer, I will try this and come back with suggestions/ideas thank you. \$\endgroup\$Johann MARTINET– Johann MARTINET2018年02月16日 15:05:01 +00:00Commented Feb 16, 2018 at 15:05
I don't know the correct nuclear physics terms, so please excuse misnomers. I am also not a Java programmer.
When I look at your Nuclide constructor I see a nested set of three classes.
- An overall collection
- An
Element
class - A
Nuclide
class. This could also be aUserType
(VB) orstruct
(C)
Each element can contain 0 or more Nuclide
s (Ok, in real life this would be 1 or more), so I see each Element
containing a collection of Nuclide
information. The Nuclide class only contains enough information and does not repeat the static information in the Element
. The key for your Nuclide
s will be the State
, which can be (and should be) implemented as a simple Enum
.
This also gives you flexibility to hard code calls to constructors. It also allows you to build a constructor code set that can call information from an external file (noting that you don't want this at this point in time).
You can call all the relevant Nuclide
calculations from the Element
or event the collection, thus encapsulating the detail while providing a simple nterface to the user/future coder.
-
\$\begingroup\$ that's what i would suggest when reading the original question and the follow up question here... the usage of
enum
is (in my honest opinion) not a good choice! \$\endgroup\$Martin Frank– Martin Frank2018年02月13日 07:58:41 +00:00Commented Feb 13, 2018 at 7:58 -
\$\begingroup\$ Thank you for your answer ! I agree with the fact that an enum is not the best way to do it. However if I use an external file how can I check the validity of submitted parameters (Z,A,I) at compile time ? \$\endgroup\$Johann MARTINET– Johann MARTINET2018年02月15日 07:49:38 +00:00Commented Feb 15, 2018 at 7:49
-
\$\begingroup\$ @JohannMARTINET - When I opened up the possibility of using a data file for input, I was thinking of a data file you control -i.e. authorised and validated data. However, I was only noting the flexibility in my approach to adjust to future workflow if needed. I recognise this is a slow-moving field and if something changes you can either amend the relevant data held in your code or (if you set this up in the future) in your authorised data file. I would think that the work to do either would be about the same. The concepts of using the classes here can be re-used for other calculations? \$\endgroup\$AJD– AJD2018年02月15日 18:38:37 +00:00Commented Feb 15, 2018 at 18:38
-
\$\begingroup\$ @JohannMARTINET, what kind of compile time errors are we talking about? Errors when you compile your library, or errors when someone else using your library compiles their project? You already mentioned you'd like an API such as
Nuclide.get("C14")
, which cannot give compile-time errors for the party calling the get-method. \$\endgroup\$ZeroOne– ZeroOne2018年03月07日 22:50:12 +00:00Commented Mar 7, 2018 at 22:50