rsanchez-wsu / fa15-ceg3120

11 stars 2 forks source link

Ant build, commonl.xml issues #22

Closed pquackenbush closed 9 years ago

pquackenbush commented 9 years ago

So I've finally managed to get a build successful build together. However, to do this I had to comment out an import in one of the common ADTs that was referring to CustomerClient.java. I put the import back in, build fails.

On top of this, I've noticed that while all the modules have a src/build, src/tools... created. There is no common/src/build. This leads me to believe that the files in common/src are not getting compiled properly. Anyone have any ideas on how to tweak the common.xml to allow references from common to the modules?

AKA-Syenite commented 9 years ago

customerclient, contractorclient and server are the three modules that get compiled, common contains files that get compiled into all three of them. People shouldn't be importing ANYTHING from a module into a common class or from a module into a seperate module because that will break.

pquackenbush commented 9 years ago

While I understand that's how common is typically used, this breaks the common launch model that we agreed upon at the start of the semester. Something is going to need references to all three modules in order to launch one of the three UIs.

AKA-Syenite commented 9 years ago

Just use a bit of reflection to see which module you're in if you want a common UI

pquackenbush commented 9 years ago

Not going to lie. Before this, I had never even heard of reflection and introspection. Can you take a look and see if you have any pointers on what I did in that last commit. I looks like it works, but I doubt it's very safe, and probably a potential source of monster issues later.

Thanks for the tip :jack_o_lantern:

AKA-Syenite commented 9 years ago

Yeah, that should work well, although you should probably handle a null class more specifically.

pquackenbush commented 9 years ago

Would a null check be necessary? Class.forName(String) throws a ClassNotFoundException, halting execution in the method, and Class.getMethod(String, Class<?>...) throws a NoSuchMethodException, also halting execution in the method. I'm not quite sure I see where the check would be since if either call fails, an exception is thrown. With the way I have it now, I just don't see where null would even be possible.

AKA-Syenite commented 9 years ago

It's not strictly necessary, just a style thing.

pquackenbush commented 9 years ago

ok, thanks.

nathanjent commented 9 years ago

I agree, any class that can be useful to another module should be put in common. It shouldn't matter which team created it. The main class of each module can initialize whatever classes it needs and build up from there.

pquackenbush commented 9 years ago

Agreed, the issue I was having earlier was trying to reach the modules from within common. Since common wasn't getting compiled, any reference outside of the common super package was doomed to break the build. Shukaro suggested solving this with reflection instead of modding the build tasks.

AKA-Syenite commented 9 years ago

Common is compiled though, into each module. It doesn't get its own jar file.

pquackenbush commented 9 years ago

Right, common actually gets compiled 3 times. I think we're on the same page here. What I meant by not getting compiled is that it's not compiling separately like each module is. It gets compiled in each module, so it only has visibility on that current module. Any reference to a module from within common will fail in two out of the three modules. If you look in common/src after a compile/build/test, you'll see there is no build directory full of all the class files like there are in modules/.../src. As I'm sure you know because you wrote the build :stuck_out_tongue:

Regardless, I now understand a little about reflection and introspection and how to prevent imports from outside common.