Thread: Writing better Java

Page 1 of 13 12311 ... LastLast
Results 1 to 10 of 129
  1. #1 Writing better Java 


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    Brief FAQ:

    What is immutability (and mutability)?

    • Immutable means it can't be changed, mutable means it can.


    What's an enumerated type/enumerator?

    • An enumerated type is an enum, an enumerator is an element of an enum:
      Code:
      public enum EnumeratedType {
          ENUMERATOR;
      }


    What's a lambda/anonymous class?







    Accessability:

    • Make everything as localised (i.e. private) as you can.
    • Give classes that only contain constants or utility methods a private constructor.
    • Don't have internal (i.e. nested) classes or enumerated types unless they are only used by the outer class.
    • Avoid using accessors and mutators for variables in the same class, unless they perform more than simple get/set.
    • In abstract classes, make variables that may be used by subclasses protected, to avoid the above.




    Avoid anti-patterns:
    • Do not declare variables in interfaces (yes, you can do this, they're just static and final by default). This is known as the constant interface anti-pattern and it leaks the constants into the sub-classes, where they almost certainly won't always be required.
    • Don't use singletons - they suck (pretty much impossible to unit test, hard to refactor, basically global which isn't OO, ...).
    • Avoid creating methods where calling a super method is necessary (the call super anti-pattern). Constructors are obviously excluded.
    • Use enums over loads of related constants (e.g. for a class that contains skill ids).
    • Know when inner classes will be unnecessarily created, and avoid such cases, e.g.:

      Code:
      private final Map<String, String> map = new HashMap<String, String>() {
      	{
      		put("key", "value"); // XXX BAD!!
      	}
      };
      Code:
      private final Map<String, String> map = ImmutableMap.of("key", "value"); // Way better (requires Guava)
      Code:
      private final Map<String, String> map; // Non-Guava version (get Guava already)
      
      public MyClass() {
          Map<String, String> map = new HashMap<>();
          map.put("key", "value");
          this.map = map; // Alternatively use Collections.unmodifiableMap(map);
      }




    Code clarity:
    • Use hexadecimal literals when appropriate to increase code clarity (e.g. 0xFFFFFF is more intuitive than 16,777,215).
    • Use underscores as part of numeric literals (identical to how one would normally use commas), e.g. 1_000_000_000.
    • Avoid mixing bit operations (<< >> | & ~ ^ >>>) and arithmetic operations - stick to one type per line where possible.
    • Use bit operations when bitpacking so it is immediately obvious that packing is intentional.
    • Avoid unnecessarily prefixing literals with 0s (usually done with hex values, like 0x00FF - you wouldn't do it with decimals!).
    • Only clarify with the class name (for static members) or "this." (for instance members) when you have to. Using it unnecessarily makes code harder to read.
    • Know when specifying a literal (e.g. 2D) is unnecessary, and avoid using it if it is. The same goes for unnecessary decimal places (e.g. 0.0).
    • Use constant fields as much as possible (e.g. MyCoolAgilityCourse.LOG_WALKING_ANIMATION) to avoid magic constants. These will be inlined by the Java compiler (if they are simple literal values), or the JIT compiler if they are more complex, so there is no performance loss.

      Code:
      player.addItem(1238, 50); // These are just random values I picked
      player.playAnimation(672);
      player.playGraphic(918);
      Compared to:

      Code:
      player.addItem(MY_COOL_QUEST_REWARD_ITEM, 50); // Making 50 a constant is probably going overboard
      player.playAnimation(QUEST_COMPLETION_ANIMATION);
      player.playGraphic(QUEST_COMPLETION_GRAPHIC);




    Design and data structures:
    • Save object pooling for objects with large initialisation overhead (like database connections or threads). Ensure that objects are returned to an initial state when they are returned to the pool.
    • Verify you're using the correct data structure. Just need a simple map? Use HashMap, not TreeMap. Need a way to add elements to the start of a List? Try a Deque instead.
    • Be aware of the running times of various data structures - HashMap has (amortized) constant time contains, add, remove, etc, whereas ArrayList is O(n) for contains and remove. Select an appropriate one to ensure your application scales well.
    • Modularity is non-existent here (scape-emu is the only public server that practices it). Modularity and good, flexible design go hand-in-hand.
    • Know the law of Demeter and ensure your code obeys. Wrapping a Map? Add get/put methods in the wrapper class instead of exposing the map.




    Exceptions:
    • Most of the time, you probably want methods to throw an exception instead of catching it and printing a stack trace - think to yourself "should the thing calling this method know about this problem?".
    • Know the difference between checked and unchecked exceptions, and when to use each.
    • Consider using utility methods like Guava's Preconditions or the more limited Objects.checkNotNull(), for nicer and shorter code.
    • Add exceptions that may be thrown (both checked and unchecked) to the JavaDoc of a method, using the @throws tag (should be placed below the @return tag).




    General comments:
    • DON'T REWRITE CODE OTHER PEOPLE HAVE ALREADY DONE WELL. Guava is superb - get it.
    • Unit testing (JUnit). Do it.
    • If you are overriding either hashCode() or equals(), override the other one too. Not doing so violates the hashCode-equals contract (i.e. if a.equals(b) then a.hashCode() == b.hashCode(), and vice-versa) and is a total nuisance to debug when you forget.
    • Use Optional instead of null everywhere you can.
    • Keep methods short - if they're longer than ~20-30 lines, split them up.
    • Avoid arrays of generic types (use ArrayList instead) - type safety isn't guaranteed with arrays.
    • Utilise streams for some really superb one-liners.
    • Use maven/gradle and git...




    Mutability:

    • Make everything final when you can, except for parameters and local variables (only make these final when they really should not be changed). Obviously don't bother making methods final if the class is final. Use the builder pattern in situations where it initially seems nicer to make things mutable and call loads of setX methods (example implementation).
    • Consider the unmodifiable methods in Collections to create immutable variants (or better, Guava's ImmutableX classes).
    • There is no way (yet) to make an immutable array in Java (unless the array has 0 elements). Use an ArrayList instead.
    • Consider using a partial copy constructor (or method) to easily create variants of immutable data.




    Naming and commenting:
    • Adhere to Java naming conventions (and formatting too).
    • Name things appropriately!!! Don't give them ridiculous names ("a", "something"), and make sure to change the name if you change what it is/does/contains.
    • Only use UNDERSCORE_STYLE_NAMING for things that are actually constants (i.e. things that are immutable). Most objects are not immutable even if you declare the field final.
    • Add proper JavaDoc to your code (see apollo).
    • In spite of the above, let your code document itself. Adding lots of unnecessary comments makes code hard to read and ugly - reserve commenting for code that is unavoidably obtuse (e.g. the infamous you are not expected to understand this) or for explaining why some code isn't done the intuitive way (useful so that people don't change it and then wonder why there's a bug or other problem).
    • Avoid Hungarian notation (i.e. state or type indicated by name, such as IList for a list interface, or _var_ for private variables). Such notation is archaic, unnecessary, and ugly.




    Portability:
    • Unix and Linux file systems are case sensitive - meaning "Hello.txt" and "hello.txt" are not the same file. Ensure that you use the same capitalisation for e.g. file names throughout your code, or it will not run on such machines (see http://www.rune-server.org/runescape...ing-wrong.html).
    • Use platform-independent file paths - "C:/.my_cool_folder" will not work properly for anyone not on Windows. You can access many useful properties using System#getProperty, such as the home directory of the user ("user.home") - utilise those, and place whatever you want in there (be sure to prefix the folder with a . too).
    • Use relative positioning when making GUIs, web-pages, etc - not everyone will have the same monitor resolution + size as you, so what looks good to you will almost certainly look bad to at least some other users.
    • Avoid JNI. Thankfully nobody here has managed to think using JNI is a good idea, but it almost never is. If you do _have_ to use it (you almost certainly don't), use JNA where possible.




    Premature or unnecessary optimisation:
    • Don't optimise code that isn't the bottleneck (or anything like it) - see the Pareto principle.
    • Reserve lazy initialisation for very expensive objects that are only used sometimes. In the vast, vast majority of cases you should not be using it (I have personally never used it).
      Quote Originally Posted by Graham View Post
      don't set stuff to null if it's about to fall out of scope anyway
    • Avoid using StringBuilder unless your concatenation code is very complicated. The compiler will convert code that uses concat to StringBuilder for you (see StringBuilder vs Concatenation for more info).
    • For primitives only (i.e. not arrays), don't use byte or short in an attempt to minimise memory usage - int is the default numeric literal, so use it throughout. The Java compiler will compile byte and short values to ints anyway, so there is no saved memory.




    Pretty code:
    • Use lambdas instead of anonymous classes. Consider implementing function or consumer, or taking one as a parameter in cases where something can't (or shouldn't) implement one of them.
    • Use the least specific type on the LHS you can (while still keeping what the code does obvious), e.g.:
      Code:
      List<String> strings = new ArrayList<>(); // List on the LHS, not ArrayList
    • Use the diamond operator for shorter code and easy refactoring:
      Code:
      List<String> list = new ArrayList<>(); // Only need to change the generic parameter on the LHS
    • Use JavaFX over Swing or (shudder) AWT. Don't use the SceneBuilder for anything complex.
    • Consider using static methods to create objects instead of public constructors when extra clarification is needed - you can name the static method.
    • Use ARM (automatic resource management, aka try-with-resource) blocks with anything that implements AutoCloseable:
      Code:
       try (Reader reader = new BufferedReader(new FileReader("file")); // Note the parentheses and how multiple AutoCloseables
            OutputStream out = new FileOutputStream("file2")) {         // can be included in one.
          /* code here */
      } // Automatically calls close() for you here.
    • Separate reserved characters into concatenated strings - the compiler will fold them into one, so there is no performance loss (it will also do this will other literals, e.g. int i = 6 * 4 will be compiled to int i = 24).
      Code:
      String bad = "Hello\nWorld!\n";
      String good = "Hello" + "\n" + "World!" + "\n";

      Protip: Both the diamond operator and ARM are purely syntax sugar, so if you modify the version part of the class file header, you can use them with Java 5 or 6 too!




    Threading:
    • Utilise the multi-core processors everyone has nowadays. The executor framework is your friend (Runtime.getRuntime().availableProcessors() too).
    • Be careful. Mutable data shared between threads must be sychronized (pro-tip: make everything you can immutable).
    • Be aware of operations that aren't atomic (take a single operation). Such operations may be a single line of source code, but several bytecode instructions (and hence, may only execute the first instruction before switching to another thread) - the most surprising one is:
      Code:
      int i = 0;
      i += 2; // 3 operations - read, increment, store
      Consider using AtomicInteger or equivalent for atomic integer manipulation.
    • Synchronize on the most specific object you can - there's no point having a synchronized method (which is actually syntax sugar for synchronizing the entire object) if all you really need to do is synchronize access to a single variable.
    • Don't synchronize on arrays (especially multi-dimensional ones). Use a List (and utilise Collections.synchronizedList()) instead (Arrays.asList() exists if you need to convert).
    • Assignment in Java is always atomic, except for doubles and longs (because they're both 64-bit). This means that if you are using multiple threads, even if only one thread will ever be writing, you must still synchronize access to double and long values. Consider using AtomicLong, which synchronizes for you.
    Last edited by Major; 03-29-2015 at 12:19 PM.
    Reply With Quote  
     


  2. #2  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    • Make everything final when you can, except for parameters and local variables (only make these final when they really should not be changed). Obviously don't bother making methods final if the class is final. Use the builder pattern in situations where it initially seems nicer to make things mutable and call loads of setX methods.


    Note that this isn't a particularly good way of doing this as it doesn't offer the flexibility required for editing the cache, and is clearly quite verbose. However, it serves as an example of how the pattern works.

    ItemDefinitionBuilder

    Code:
    package tld.removed.old.fs.archive.config.build;
    
    import java.util.ArrayList;
    import java.util.Arrays;
    import java.util.HashMap;
    import java.util.List;
    import java.util.Map;
    
    import tld.removed.old.fs.archive.build.ConfigBuilder;
    import tld.removed.old.fs.archive.config.ItemDefinition;
    
    /**
     * A {@link ConfigBuilder} for {@link ItemDefinition}s.
     * 
     * @author Major
     */
    public final class ItemDefinitionBuilder extends ConfigBuilder<ItemDefinition> {
    
    	/**
    	 * The default equipment model ids.
    	 */
    	protected static final List<Integer> DEFAULT_EQUIP_IDS = Arrays.asList(-1, -1, -1);
    
    	/**
    	 * The default ground scale values.
    	 */
    	protected static final List<Integer> DEFAULT_GROUND_SCALES = Arrays.asList(128, 128, 128);
    
    	/**
    	 * The default head piece ids.
    	 */
    	protected static final List<Integer> DEFAULT_HEAD_PIECES = Arrays.asList(-1, -1);
    
    	/**
    	 * The description of this item.
    	 */
    	protected String description = "null";
    
    	/**
    	 * The female primary, secondary, and tertiary equipment ids of this item.
    	 */
    	protected List<Integer> femaleEquipIds = new ArrayList<>(DEFAULT_EQUIP_IDS);
    
    	/**
    	 * The y-translation for the female equipment piece of this item.
    	 */
    	protected int femaleEquipTranslation = 0;
    
    	/**
    	 * The female head piece ids.
    	 */
    	protected List<Integer> femaleHeadPieces = new ArrayList<>(DEFAULT_HEAD_PIECES);
    
    	/**
    	 * The ground menu actions of this item.
    	 */
    	protected List<String> groundMenuActions = new ArrayList<>(5);
    
    	/**
    	 * The x, y, and z ground scale values of this item.
    	 */
    	protected List<Integer> groundScales = new ArrayList<>(DEFAULT_GROUND_SCALES);
    
    	/**
    	 * The inventory menu actions of this item.
    	 */
    	protected List<String> inventoryMenuActions = new ArrayList<>(5);
    
    	/**
    	 * The light ambience of this item.
    	 */
    	protected int lightAmbience = 0;
    
    	/**
    	 * The light diffusion of this item.
    	 */
    	protected int lightDiffusion = 0;
    
    	/**
    	 * The male primary, secondary, and tertiary equipment ids of this item.
    	 */
    	protected List<Integer> maleEquipIds = new ArrayList<>(DEFAULT_EQUIP_IDS);
    
    	/**
    	 * The y-translation for the male equipment piece of this item.
    	 */
    	protected int maleEquipTranslation = 0;
    
    	/**
    	 * The male head piece ids.
    	 */
    	protected List<Integer> maleHeadPieces = new ArrayList<>(DEFAULT_HEAD_PIECES);
    
    	/**
    	 * The members flag of this item.
    	 */
    	protected boolean members = false;
    
    	/**
    	 * The id of this item's model.
    	 */
    	protected int modelId = 0;
    
    	/**
    	 * The name of this item.
    	 */
    	protected String name = "null";
    
    	/**
    	 * The note info id of this item.
    	 */
    	protected int noteInfoId = -1;
    
    	/**
    	 * The note template id of this item.
    	 */
    	protected int noteTemplateId = -1;
    
    	/**
    	 * The map of original model colours to replacement colours for this item.
    	 */
    	protected Map<Integer, Integer> replacementColours = new HashMap<>(3);
    
    	/**
    	 * The camera roll of this item's sprite.
    	 */
    	protected int spriteCameraRoll = 0;
    
    	/**
    	 * The sprite camera yaw of this item.
    	 */
    	protected int spriteCameraYaw = 0;
    
    	/**
    	 * The pitch of this item's sprite.
    	 */
    	protected int spritePitch = 0;
    
    	/**
    	 * The scale of this item's sprite.
    	 */
    	protected int spriteScale = 2000;
    
    	/**
    	 * The x-translation of this item's sprite.
    	 */
    	protected int spriteTranslateX = 0;
    
    	/**
    	 * The y-translation of this item's sprite.
    	 */
    	protected int spriteTranslateY = 0;
    
    	/**
    	 * The stackable flag of this item.
    	 */
    	protected boolean stackable = false;
    
    	/**
    	 * The stack amounts of this item.
    	 */
    	protected List<Integer> stackAmounts = new ArrayList<>();
    
    	/**
    	 * The stack model ids of this item.
    	 */
    	protected List<Integer> stackModelIds = new ArrayList<>();
    
    	/**
    	 * The team id of this item.
    	 */
    	protected int team = 0;
    
    	/**
    	 * The value of this item.
    	 */
    	protected int value = 1;
    
    	/**
    	 * Creates the item definition builder.
    	 * 
    	 * @param id The id of the item.
    	 */
    	protected ItemDefinitionBuilder(int id) {
    		super(id);
    	}
    
    	/**
    	 * Builds and returns {@link ItemDefinition}.
    	 * 
    	 * @return The item definition.
    	 */
    	@Override
    	public ItemDefinition build() {
    		return new ItemDefinition(description, femaleEquipIds, femaleEquipTranslation, femaleHeadPieces,
    				groundMenuActions, groundScales, id, inventoryMenuActions, lightAmbience, lightDiffusion, maleEquipIds,
    				maleEquipTranslation, maleHeadPieces, members, modelId, name, noteInfoId, noteTemplateId,
    				replacementColours, spriteCameraRoll, spriteCameraYaw, spritePitch, spriteScale, spriteTranslateX,
    				spriteTranslateY, stackable, stackAmounts, stackModelIds, team, value);
    	}
    
    	/**
    	 * Sets the description of this item.
    	 *
    	 * @param description The description.
    	 */
    	public final void setDescription(String description) {
    		this.description = description;
    	}
    
    	/**
    	 * Sets a female equip id of this item.
    	 *
    	 * @param index The index.
    	 * @param femaleEquipId The female equip id.
    	 */
    	public final void setFemaleEquipId(int index, int femaleEquipId) {
    		femaleEquipIds.add(femaleEquipId);
    	}
    
    	/**
    	 * Sets the female equip translation of this item.
    	 *
    	 * @param femaleEquipTranslation The female equip translation.
    	 */
    	public final void setFemaleEquipTranslation(int femaleEquipTranslation) {
    		this.femaleEquipTranslation = femaleEquipTranslation;
    	}
    
    	/**
    	 * Sets a ground scale value. TODO use an enum for direction.
    	 * 
    	 * @param direction The direction.
    	 * @param scale The scale.
    	 */
    	public final void setGroundScale(int direction, int scale) {
    		groundScales.add(scale);
    	}
    
    	/**
    	 * Sets a female head piece of the item.
    	 *
    	 * @param index The index
    	 * @param femaleHeadPiece The female head piece.
    	 */
    	public final void setFemaleHeadPiece(int index, int femaleHeadPiece) {
    		femaleHeadPieces.add(femaleHeadPiece);
    	}
    
    	/**
    	 * Sets the ground menu action at the specified index.
    	 * 
    	 * @param index The index.
    	 * @param action The action string.
    	 */
    	public final void setGroundAction(int index, String action) {
    		groundMenuActions.add(action);
    	}
    
    	/**
    	 * Sets the inventory menu action at the specified index.
    	 * 
    	 * @param index The action index.
    	 * @param action The action string.
    	 */
    	public final void setInventoryAction(int index, String action) {
    		inventoryMenuActions.add(action);
    	}
    
    	/**
    	 * Sets the light ambience of this item.
    	 *
    	 * @param lightAmbience The light ambience.
    	 */
    	public final void setLightAmbience(int lightAmbience) {
    		this.lightAmbience = lightAmbience;
    	}
    
    	/**
    	 * Sets the light diffusion of this item.
    	 *
    	 * @param lightDiffusion The light diffusion.
    	 */
    	public final void setLightDiffusion(int lightDiffusion) {
    		this.lightDiffusion = lightDiffusion;
    	}
    
    	/**
    	 * Sets a male equip id of this item.
    	 *
    	 * @param index The index.
    	 * @param maleEquipId The male equip id.
    	 */
    	public final void setMaleEquipId(int index, int maleEquipId) {
    		maleEquipIds.add(maleEquipId);
    	}
    
    	/**
    	 * Sets the male equip translation of this item.
    	 *
    	 * @param maleEquipTranslation The male equip translation.
    	 */
    	public final void setMaleEquipTranslation(int maleEquipTranslation) {
    		this.maleEquipTranslation = maleEquipTranslation;
    	}
    
    	/**
    	 * Sets a male head piece of the item.
    	 *
    	 * @param maleHeadPiece The male head piece.
    	 */
    	public final void setMaleHeadPiece(int index, int maleHeadPiece) {
    		maleHeadPieces.add(maleHeadPiece);
    	}
    
    	/**
    	 * Sets the members flag of this item.
    	 *
    	 * @param members The members flag.
    	 */
    	public final void setMembers(boolean members) {
    		this.members = members;
    	}
    
    	/**
    	 * Sets the model id of this item.
    	 * 
    	 * @param modelId The model id.
    	 */
    	public final void setModelId(int modelId) {
    		this.modelId = modelId;
    	}
    
    	/**
    	 * Sets the name of this item.
    	 *
    	 * @param name The name.
    	 */
    	public final void setName(String name) {
    		this.name = name;
    	}
    
    	/**
    	 * Sets the note info id of this item.
    	 *
    	 * @param noteInfoId The note info id.
    	 */
    	public final void setNoteInfoId(int noteInfoId) {
    		this.noteInfoId = noteInfoId;
    	}
    
    	/**
    	 * Sets the note template id of this item.
    	 *
    	 * @param noteTemplateId The note template id.
    	 */
    	public final void setNoteTemplateId(int noteTemplateId) {
    		this.noteTemplateId = noteTemplateId;
    	}
    
    	/**
    	 * Sets the replacement model colours of this item.
    	 *
    	 * @param replacementColours The replacement model colours.
    	 */
    	public final void setReplacementColours(Map<Integer, Integer> replacementColours) {
    		this.replacementColours = replacementColours;
    	}
    
    	/**
    	 * Sets the sprite camera roll of this item.
    	 *
    	 * @param spriteCameraRoll The sprite camera roll.
    	 */
    	public final void setSpriteCameraRoll(int spriteCameraRoll) {
    		this.spriteCameraRoll = spriteCameraRoll;
    	}
    
    	/**
    	 * Sets the sprite camera yaw of this item.
    	 *
    	 * @param spriteCameraYaw The sprite camera yaw.
    	 */
    	public final void setSpriteCameraYaw(int spriteCameraYaw) {
    		this.spriteCameraYaw = spriteCameraYaw;
    	}
    
    	/**
    	 * Sets the sprite pitch of this item.
    	 *
    	 * @param spritePitch The sprite pitch.
    	 */
    	public final void setSpritePitch(int spritePitch) {
    		this.spritePitch = spritePitch;
    	}
    
    	/**
    	 * Sets the sprite scale of this item.
    	 *
    	 * @param spriteScale The sprite scale.
    	 */
    	public final void setSpriteScale(int spriteScale) {
    		this.spriteScale = spriteScale;
    	}
    
    	/**
    	 * Sets the sprite x translation of this item.
    	 *
    	 * @param spriteTranslateX The sprite x translation.
    	 */
    	public final void setSpriteTranslateX(int spriteTranslateX) {
    		this.spriteTranslateX = spriteTranslateX;
    	}
    
    	/**
    	 * Sets the sprite y translation of this item.
    	 *
    	 * @param spriteTranslateY The sprite y translation.
    	 */
    	public final void setSpriteTranslateY(int spriteTranslateY) {
    		this.spriteTranslateY = spriteTranslateY;
    	}
    
    	/**
    	 * Sets the stackable flag of this item.
    	 *
    	 * @param stackable The stackable flag.
    	 */
    	public final void setStackable(boolean stackable) {
    		this.stackable = stackable;
    	}
    
    	/**
    	 * Sets a stack amount of this item.
    	 *
    	 * @param index The index.
    	 * @param stackAmount The stack amount.
    	 */
    	public final void setStackAmount(int index, int stackAmount) {
    		stackAmounts.add(stackAmount);
    	}
    
    	/**
    	 * Sets a stack model id of this item.
    	 *
    	 * @param index The index.
    	 * @param stackModelId The stack model id.
    	 */
    	public final void setStackModelIds(int index, int stackModelId) {
    		stackModelIds.add(stackModelId);
    	}
    
    	/**
    	 * Sets the team of this item.
    	 *
    	 * @param team The team.
    	 */
    	public final void setTeam(int team) {
    		this.team = team;
    	}
    
    	/**
    	 * Sets the value of this item.
    	 *
    	 * @param value The value.
    	 */
    	public final void setValue(int value) {
    		this.value = value;
    	}
    
    }
    ItemDefinition
    Code:
    package tld.removed.old.fs.archive.config;
    
    import java.util.List;
    import java.util.Map;
    
    import tld.removed.old.fs.archive.Definition;
    
    /**
     * A definition for an item ('object'). Note that the model-related stuff should really be in a separate class (see the
     * massive constructor of this one), but is kept here for the example.
     * 
     * @author Major
     */
    public final class ItemDefinition extends Definition {
    
    	/**
    	 * The description of this item.
    	 */
    	private final String description;
    
    	/**
    	 * The female primary, secondary, and tertiary equipment ids of this item.
    	 */
    	private final List<Integer> femaleEquipIds;
    
    	/**
    	 * The y-translation for the female equipment piece of this item.
    	 */
    	private final int femaleEquipTranslation;
    
    	/**
    	 * The female head piece ids.
    	 */
    	private final List<Integer> femaleHeadPieces;
    
    	/**
    	 * The ground menu actions of this item.
    	 */
    	private final List<String> groundMenuActions;
    
    	/**
    	 * The x, y, and z ground scales of this item.
    	 */
    	private final List<Integer> groundScales;
    
    	/**
    	 * The inventory menu actions of this item.
    	 */
    	private final List<String> inventoryMenuActions;
    
    	/**
    	 * The light ambience of this item.
    	 */
    	private final int lightAmbience;
    
    	/**
    	 * The light diffusion of this item.
    	 */
    	private final int lightDiffusion;
    
    	/**
    	 * The male primary, secondary, and tertiary equipment ids of this item.
    	 */
    	private final List<Integer> maleEquipIds;
    
    	/**
    	 * The y-translation for the male equipment piece of this item.
    	 */
    	private final int maleEquipTranslation;
    
    	/**
    	 * The male head piece ids.
    	 */
    	private final List<Integer> maleHeadPieces;
    
    	/**
    	 * The members flag of this item.
    	 */
    	private final boolean members;
    
    	/**
    	 * The id of this item's model.
    	 */
    	private final int modelId;
    
    	/**
    	 * The name of this item.
    	 */
    	private final String name;
    
    	/**
    	 * The note info id of this item.
    	 */
    	private final int noteInfoId;
    
    	/**
    	 * The noted template id of this item.
    	 */
    	private final int noteTemplateId;
    
    	/**
    	 * The map of original model colours to their replacements for this item.
    	 */
    	private final Map<Integer, Integer> replacementColours;
    
    	/**
    	 * The camera roll of this item's sprite.
    	 */
    	private final int spriteCameraRoll;
    
    	/**
    	 * The sprite camera yaw of this item.
    	 */
    	private final int spriteCameraYaw;
    
    	/**
    	 * The pitch of this item's sprite.
    	 */
    	private final int spritePitch;
    
    	/**
    	 * The scale of this item's sprite.
    	 */
    	private final int spriteScale;
    
    	/**
    	 * The x-translation of this item's sprite.
    	 */
    	private final int spriteTranslateX;
    
    	/**
    	 * The y-translation of this item's sprite.
    	 */
    	private final int spriteTranslateY;
    
    	/**
    	 * The stackable flag of this item.
    	 */
    	private final boolean stackable;
    
    	/**
    	 * The stack amounts of this item.
    	 */
    	private final List<Integer> stackAmounts;
    
    	/**
    	 * The stack model ids of this item.
    	 */
    	private final List<Integer> stackModelIds;
    
    	/**
    	 * The team id of this item.
    	 */
    	private final int team;
    
    	/**
    	 * The value of this item.
    	 */
    	private final int value;
    
    	/**
    	 * Creates the item definition. See the comment at the top of this class if you agree that it is stupidly big.
    	 */
    	public ItemDefinition(String description, List<Integer> femaleEquipIds, int femaleEquipTranslation,
    			List<Integer> femaleHeadPieces, List<String> groundMenuActions, List<Integer> groundScales, int id,
    			List<String> inventoryMenuActions, int lightAmbience, int lightDiffusion, List<Integer> maleEquipIds,
    			int maleEquipTranslation, List<Integer> maleHeadPieces, boolean members, int modelId, String name,
    			int noteInfoId, int noteTemplateId, Map<Integer, Integer> replacementColours, int spriteCameraRoll,
    			int spriteCameraYaw, int spritePitch, int spriteScale, int spriteTranslateX, int spriteTranslateY,
    			boolean stackable, List<Integer> stackAmounts, List<Integer> stackModelIds, int team, int value) {
    		this.description = description;
    		this.femaleEquipIds = femaleEquipIds;
    		this.femaleEquipTranslation = femaleEquipTranslation;
    		this.femaleHeadPieces = femaleHeadPieces;
    		this.groundMenuActions = groundMenuActions;
    		this.groundScales = groundScales;
    		this.inventoryMenuActions = inventoryMenuActions;
    		this.lightAmbience = lightAmbience;
    		this.lightDiffusion = lightDiffusion;
    		this.maleEquipIds = maleEquipIds;
    		this.maleEquipTranslation = maleEquipTranslation;
    		this.maleHeadPieces = maleHeadPieces;
    		this.members = members;
    		this.modelId = modelId;
    		this.name = name;
    		this.noteInfoId = noteInfoId;
    		this.noteTemplateId = noteTemplateId;
    		this.replacementColours = replacementColours;
    		this.spriteCameraRoll = spriteCameraRoll;
    		this.spriteCameraYaw = spriteCameraYaw;
    		this.spritePitch = spritePitch;
    		this.spriteScale = spriteScale;
    		this.spriteTranslateX = spriteTranslateX;
    		this.spriteTranslateY = spriteTranslateY;
    		this.stackable = stackable;
    		this.stackAmounts = stackAmounts;
    		this.stackModelIds = stackModelIds;
    		this.team = team;
    		this.value = value;
    	}
    
    	/**
    	 * Gets the description.
    	 *
    	 * @return The description.
    	 */
    	public String getDescription() {
    		return description;
    	}
    
    	/**
    	 * Gets the {@link List} of female equip ids.
    	 *
    	 * @return The female equip ids.
    	 */
    	public List<Integer> getFemaleEquipIds() {
    		return femaleEquipIds;
    	}
    
    	/**
    	 * Gets the female equip translation.
    	 *
    	 * @return The female equip translation.
    	 */
    	public int getFemaleEquipTranslation() {
    		return femaleEquipTranslation;
    	}
    
    	/**
    	 * Gets the {@link List} of female head pieces.
    	 *
    	 * @return The female head pieces.
    	 */
    	public List<Integer> getFemaleHeadPieces() {
    		return femaleHeadPieces;
    	}
    
    	/**
    	 * Gets the {@link List} of ground menu actions.
    	 *
    	 * @return The ground menu actions.
    	 */
    	public List<String> getGroundMenuActions() {
    		return groundMenuActions;
    	}
    
    	/**
    	 * Gets the {@link List} of ground scale values.
    	 *
    	 * @return The ground scale values.
    	 */
    	public List<Integer> getGroundScales() {
    		return groundScales;
    	}
    
    	/**
    	 * Gets the {@link List} of inventory menu actions.
    	 *
    	 * @return The inventory menu actions.
    	 */
    	public List<String> getInventoryMenuActions() {
    		return inventoryMenuActions;
    	}
    
    	/**
    	 * Gets the light ambience.
    	 *
    	 * @return The light ambience.
    	 */
    	public int getLightAmbience() {
    		return lightAmbience;
    	}
    
    	/**
    	 * Gets the light diffusion.
    	 *
    	 * @return The light diffusion.
    	 */
    	public int getLightDiffusion() {
    		return lightDiffusion;
    	}
    
    /**
    		 * Gets the {@link List] of male equip ids.
    		 *
    		 * @return The male equip ids.
    		 */
    	public List<Integer> getMaleEquipIds() {
    		return maleEquipIds;
    	}
    
    	/**
    	 * Gets the male equip translation.
    	 *
    	 * @return The male equip translation.
    	 */
    	public int getMaleEquipTranslation() {
    		return maleEquipTranslation;
    	}
    
    	/**
    	 * Gets the {@link List} of male head pieces.
    	 *
    	 * @return The male head pieces.
    	 */
    	public List<Integer> getMaleHeadPieces() {
    		return maleHeadPieces;
    	}
    
    	/**
    	 * Gets the model id.
    	 *
    	 * @return The model id.
    	 */
    	public int getModelId() {
    		return modelId;
    	}
    
    	/**
    	 * Gets the name.
    	 *
    	 * @return The name.
    	 */
    	public String getName() {
    		return name;
    	}
    
    	/**
    	 * Gets the note info id.
    	 *
    	 * @return The note info id.
    	 */
    	public int getNoteInfoId() {
    		return noteInfoId;
    	}
    
    	/**
    	 * Gets the note template id.
    	 *
    	 * @return The note template id.
    	 */
    	public int getNoteTemplateId() {
    		return noteTemplateId;
    	}
    
    	/**
    	 * Gets the {@link Map} of original model colours to replacement colours.
    	 *
    	 * @return The replacement colours.
    	 */
    	public Map<Integer, Integer> getReplacementColours() {
    		return replacementColours;
    	}
    
    	/**
    	 * Gets the sprite camera roll.
    	 *
    	 * @return The sprite camera roll.
    	 */
    	public int getSpriteCameraRoll() {
    		return spriteCameraRoll;
    	}
    
    	/**
    	 * Gets the sprite camera yaw.
    	 *
    	 * @return The sprite camera yaw.
    	 */
    	public int getSpriteCameraYaw() {
    		return spriteCameraYaw;
    	}
    
    	/**
    	 * Gets the sprite pitch.
    	 *
    	 * @return The sprite pitch.
    	 */
    	public int getSpritePitch() {
    		return spritePitch;
    	}
    
    	/**
    	 * Gets the sprite scale.
    	 *
    	 * @return The sprite scale.
    	 */
    	public int getSpriteScale() {
    		return spriteScale;
    	}
    
    	/**
    	 * Gets the sprite translate x.
    	 *
    	 * @return The sprite translate x.
    	 */
    	public int getSpriteTranslateX() {
    		return spriteTranslateX;
    	}
    
    	/**
    	 * Gets the sprite translate y.
    	 *
    	 * @return The sprite translate y.
    	 */
    	public int getSpriteTranslateY() {
    		return spriteTranslateY;
    	}
    
    	/**
    	 * Gets the {@link List} of stack amounts.
    	 *
    	 * @return The stack amounts.
    	 */
    	public List<Integer> getStackAmounts() {
    		return stackAmounts;
    	}
    
    	/**
    	 * Gets the {@link List} of stack model ids.
    	 *
    	 * @return The stack model ids.
    	 */
    	public List<Integer> getStackModelIds() {
    		return stackModelIds;
    	}
    
    	/**
    	 * Gets the team.
    	 *
    	 * @return The team.
    	 */
    	public int getTeam() {
    		return team;
    	}
    
    	/**
    	 * Gets the value.
    	 *
    	 * @return The value.
    	 */
    	public int getValue() {
    		return value;
    	}
    
    	/**
    	 * Gets the members flag.
    	 *
    	 * @return {@code true} if this item is members only, otherwise {@code false}.
    	 */
    	public boolean isMembers() {
    		return members;
    	}
    
    	/**
    	 * Gets the stackable.
    	 *
    	 * @return The stackable.
    	 */
    	public boolean isStackable() {
    		return stackable;
    	}
    
    }
    Last edited by Major; 09-01-2014 at 04:03 PM.
    Reply With Quote  
     


  3. #3  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    • Use switch sparingly (e.g. for enumerated types). If-else is shorter, usually faster, and quicker to write (those pesky break; statements). It's ideal when you want to case-sensitively match strings though - much nicer than loads of equals() calls. Be aware that long chains of if-elseif can make code more difficult to read - consider abstracting if you find this happening (although sometimes it's not possible, e.g. when reading data from the cache).


    Why are if statements often faster than switch statements?

    1. Branch prediction. Modern CPUs have a 90-95% success rate for this (not the 50% you might expect). See this StackOverflow question for a decent explanation.

    2. Unless the cases in the switch are very similar (e.g. 1, 2, and 4, or 768, 769, and 772 - not both), the compiler will output the lookupswitch instruction. This is actually O(log n) rather than the O(1) (constant time) people generally assume switch provides (see Big O notation if you don't understand what this means - tl;dr O(log n) is slower). If the values are similar, or you are switching over the majority of enumerators in an enumerated type, then the tableswitch instruction is emitted instead, which is O(1).

    Switching over an enumerated type is really just syntactic sugar that the compiler changes to switching over the enum ordinal (i.e. it's identical to the above way).

    Note that O(log n) isn't exactly slow - it's just going to be slower than constant time (not accounting for possible constants which aren't included - if you aren't entirely sure what this means, ignore it).

    Table switch:
    Code:
    switch (i) {
    case 0:
    	System.out.println("i is 0!");
    	break;
    case 1:
    	System.out.println("i is 1!");
    	break;
    case 3:
    	System.out.println("i is 3!");
    	break;
    }
    Lookupswitch
    Code:
    switch (i) {
    case 0:
    	System.out.println("i is 0!");
    	break;
    case 42:
    	System.out.println("i is 42!");
    	break;
    case 673:
    	System.out.println("i is 673!");
    	break;
    }
    Bytecode (kinda):
    This isn't particularly useful if you're interested in the above (although note how tableswitch has a range but lookupswitch has a case count) - it's just for people who are more interested.

    Tableswitch:
    Code:
    iload_0
    tableswitch 0 to 3 // case range
        0:  32 (+31) // Jump to instruction 31
        1:  43 (+42)
        2:  62 (+61) // <-- Note how 2 is added by the compiler (it goes to the return statement at the end - does nothing).
        3:  54 (+53)
        default:  62 (+61) // the return instruction
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #22 // load constant #22 ("i is 0!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    goto 62 (+22) // the return instruction
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #30 // load constant #30 ("i is 1!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    goto 62 (+11) // the return instruction
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #32 // load constant #32 ("i is 3!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    
    return // instruction 62
    N.b. tableswitch takes an offset so that enumerators which are collectively similar, but not close to 0, will still work (e.g. switching over 672, 673, and 675 will give an offset of 672).

    Lookupswitch:
    Code:
    iload_0
    lookupswitch 3 // case count
        0:  36 (+35) // jump to instruction 36
        42:  47 (+46)
        673:  58 (+57) // 1, 2, .. 41 aren't added; instead, (in the jvm) a sorted array stores the data
        default:  66 (+65)
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #22 // load constant #22 ("i is 0!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    goto 66 (+22) // the return instruction
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #38 // load constant #38 ("i is 42!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    goto 66 (+11) // the return instruction
    
    getstatic #16 <java/lang/System/out Ljava/io/PrintStream;>
    ldc #40 // load constant #40 ("i is 673!")
    invokevirtual #24 <java/io/PrintStream/println(Ljava/lang/String;)V>
    return // instruction 66
    Last edited by Major; 09-01-2014 at 04:15 PM.
    Reply With Quote  
     


  4. #4  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    • Utilise type inference to reduce warning suppression and make code pretty:


    Consider a class containing settings for a program, consisting of a map of setting names to a custom class, Property, like so:

    Code:
    public class Settings {
    
        private final Map<String, Property<?>> settings = new HashMap<>();
    
        public Property<?> getSetting(String name) {
            return settings.get(name);
        }
    
        public void putSetting(String name, Property<?> property) {
            settings.put(name, property);
        }
    
    }
    To get a setting, you use the getSetting method - to get an integer Property, the following is required:

    Code:
    @SuppressWarnings("unchecked")
    Property<Integer> someIntegerProperty = (Property<Integer>) settings.getSetting("someInteger");
    The cast and warning supression is required everywhere getSetting is called - rather verbose. However, we can change the getSetting method to infer the type from where it's called:

    Code:
    @SuppressWarnings("unchecked")
    public <T> void getSetting(String name) {
        return (Property<T>) settings.get(name);
    }
    Our accessing code now becomes:

    Code:
    Property<Integer> someIntegerProperty = settings.getSetting("someInteger");
    No casting or warning supression.

    Be aware that this does not prevent the potential ClassCastException if an incorrect property type is specified - it will just be thrown inside the getSetting() method instead.



    Type inference can also be used in other places, such as when creating a new instance (constructors are really just a special type of method, but whatever):

    Code:
    public class Property<T> { // T is the class type this property contains
    
        private final T value;
    
        public Property(T value) {
            this.value = value;
        }
    
    }
    If we want to add a new property to the settings directly (for example, a default setting), we can use type inference to do something like:

    Code:
    public void addDefaults(Settings settings) {
        settings.putSetting("someInteger", new Property<>("pretend this is an int");
    }                                                  // infers the type so we don't need to do 'new Property<String>("...")'



    For a type safe alternative, consider a custom map implementation of the style:

    Code:
    AttributeMap<AttributeKey<T>, Attribute<T>>
    (see Netty's AttributeMap).
    Last edited by Major; 12-21-2014 at 06:13 PM.
    Reply With Quote  
     


  5. #5  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    • Consider using a partial copy constructor (or method) to easily create variants of immutable data.


    What's a copy constructor?

    A copy constructor is a type of conversion constructor that uses an object of the same class as a base.

    What's a conversion constructor?

    A constructor that takes an object (the 'base') as a constructor parameter, and initialises the new object using properties from the specified base object.


    Copy constructors will be familiar for any C++ programmers, but in Java they're well-known as part of the Collections API:

    Code:
    List<String> strings = Arrays.asList("Hello, ", "World!");
    List<string> stringsCopy = new ArrayList<>(strings); // This utilises the copy constructor
    stringCopy.forEach(System.out::println); // print each string in "stringsCopy"
    Prints:
    Code:
    Hello, 
    World! // stringsCopy contains "Hello, " and "World!", like "strings"


    Partial copy constructors can also be useful for immutable classes (although they can also be implemented as static utility methods that return the created object - the best choice is application-dependent, although makes little difference in practice).

    Consider the (a)typical Item class:

    Code:
    public final class Item {
    
        private final int id;
        private final int amount; // Avoid multiple declarations per line for non-local variables, as they cannot be documented.
    
        public Item(int id, int amount);
            this.id = id;
            this.amount = amount;
        }
    
        // ... accessors here
    
    }
    As they should be, both fields (id and amount) are immutable. Of course, this makes updating the amount of an item rather verbose. Consider implementing one of the following:

    Code:
    public Item(Item base, int amount) {
        Preconditions.checkArgument(amount > 0, "Item amount must be greater than 0."); // Guava
        this.id = base.id;
        this.amount = amount;
    }
    or

    Code:
    public static Item update(Item base, int amount) {
        Preconditions.checkArgument(amount > 0, "Item amount must be greater than 0."); // Guava
        return new Item(base.getId(), amount);
    }
    Updating the amount then becomes:

    Code:
    Item first = new Item(995, 10); // consider a way of deriving an id from a name in practice
    Item second = new Item(first, 25);
     // alternatively "Item second = Item.update(first, 25);" if the static method is used
    
    System.out.println("First item amount=" + first.getAmount() + ", second item amount=" + second.getAmount());
    => First item amount=10, second item amount=25
    The second version is ideal if any logic is required prior to creation (logic in a constructor should be discouraged), or if a class already has a considerable amount of constructors - too many can make classes considerably more difficult to read, and more difficult to identify what the most appropriate constructor for a situation may be.
    Last edited by Major; 09-02-2014 at 04:51 PM.
    Reply With Quote  
     


  6. #6  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    • Separate reserved characters into concatenated strings - the compiler will fold them into one, so there is no performance loss (it will also do this with other literals, e.g. int i = 6 * 4 will be compiled to int i = 24).


    Constant folding is the simple process of converting chains of literals (see above) into one at compile time.

    The rest of this post is just a demonstration of Javac performing constant folding, and doesn't need to be read unless you're interested in that stuff.

    As an example, consider the following code:

    Code:
    		String string_folding = "Hello" + "\t" + ", " + " World!" + "\t"; // "\t" inserts a tab
    		int int_folding = 6 * 11 - 2 + 3 / 2 + 42 * 4 % 19;
    		double double_folding = 6 * 1.5 / 3.5;
    Here is the equivalent bytecode:

    Code:
    ldc "Hello	,  World!	" // Load the specified constant ("Hello	,  World!	")
    astore_1 // Store the above string
    bipush 81 // This pushes 81 on to the stack - 81 is the result of (int) (6 * 11 - 2 + 3 / 2 + 42 * 4 % 19)
    istore_2
    ldc2_w 2.5714285714285716 // This is the result of 6 * 1.5 / 3.5
    dstore_3
    As you can see, the literals have clearly been folded.


    • Avoid using StringBuilder unless your concatenation code is very complicated. The compiler will convert code that uses concat to StringBuilder for you (see StringBuilder vs Concatenation for more info).


    When the values aren't string literals, the compiler will still optimise concatenation by inserting a StringBuilder - concatenation is expensive because it requires creating a new String object for each concatenation (which isn't too bad when there are two or three, but with more it rapidly becomes expensive). Using the StringBuilder, values can be appended inexpensively.

    Re-using part of the code from the example above:

    Code:
    	int int_folding = 6 * 11 - 2 + 3 / 2 + 42 * 4 % 19;
    	double double_folding = 6 * 1.5 / 3.5;
    	String string_folding = "Hello, " + int_folding + ", " + double_folding + " World!";
    Bytecode:

    Code:
    bipush 81
    istore_1 // Store our value of 81
    ldc2_w 2.5714285714285716
    dstore_2 // Store the folded version of 6 * 1.5 / 3.5;
    new java/lang/StringBuilder
    dup
    ldc "Hello, " // Load the first string literal
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a new StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder; // Append the value of int_folding, 81
    ldc ", "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append ", "
    dload_2
    invokevirtual java/lang/StringBuilder/append(D)Ljava/lang/StringBuilder; // Append the folded version of 6 * 1.5 / 3.5
    ldc " World!"
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append " World!"
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String; // Build the string
    astore 4 // Store the result of toString()



    Note that it may be worthwhile doing this manually (a rare situation where it may be best to optimise yourself instead of the compiler) when:
    • You are appending to a string in a large loop, AND
    • Lots of the values to be appended are returned from methods that use concatenation


    Consider the following (ridiculous) code:

    N.b. actually modifying 'i' has been omitted for brevity, so this exact code will run forever (or until the JVM runs out of memory). Pretend it doesn't :-)
    Code:
    	public void run() {
    		int i = 10;
    		String log = "";
    
    		while (i < 1_000_000) { // Iterate a lot of times
    			log += multiplyBy2(i);
    			log += add(i, 3);
                 		log += halve(i);
    		}
    	}
    
    	public String multiplyBy2(int value) { // Name for example purposes only
    		return "Multiplying " + value + " by 2.";
    	}
    
    	public String add(int a, int b) {
    		return "Adding " + a + " and " + b + ".";
    	}
    
    	public String halve(int a) {
    		return "Dividing " + a + " by 2.";
    	}
    As stated above, the compiler will helpfully use a StringBuilder to avoid the expensive concat calls. However, it won't rewrite existing methods to save object creation (i.e. a new StringBuilder object will be created for each multiplyBy2, add, and halve call - the compiler will not edit the method to take a StringBuilder parameter and pass it, because it would break code that uses reflection).


    This is slightly more complicated...

    Bytecode for the run() method:
    Code:
    bipush 10
    istore_1 // This is the 'i' variable, with the above value (10)
    ldc ""
    astore_2  // The "log" variable - currently empty
    goto 40 // Jump to instruction 40, which checks if i < 1,000,000
    new java/lang/StringBuilder
    dup
    aload_2
    invokestatic java/lang/String/valueOf(Ljava/lang/Object;)Ljava/lang/String;
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a StringBuilder
    aload_0
    iload_1
    invokevirtual Generic/multiplyBy2(I)Ljava/lang/String; // call the multiplyBy2 method
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append the result of triple to the builder
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String; // Build the string
    astore_2 // Set "log" to the built string
    new java/lang/StringBuilder
    dup
    aload_2
    invokestatic java/lang/String/valueOf(Ljava/lang/Object;)Ljava/lang/String;
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise another builder
    aload_0
    iload_1
    iconst_3
    invokevirtual Generic/add(II)Ljava/lang/String; // Call the add method
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append the result of add
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String; // Build the string
    astore_2 // Store the built string in "log"
    new java/lang/StringBuilder
    dup
    aload_2
    invokestatic java/lang/String/valueOf(Ljava/lang/Object;)Ljava/lang/String;
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a third(!) builder
    aload_0
    iload_1
    invokevirtual Generic/halve(I)Ljava/lang/String; // Call the halve method
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append the result of halve to the builder
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String; // Build the string
    astore_2 // Store it in log
    iload_1 // Load the value of our 'i' variable
    ldc 1000000 // Load 1,000,000 - used in our while() condition
    if_icmplt 6 // Jump back to instruction 6 (i.e. restart the loop) if i < 1,000,000
    That is correct - 3 string builders were created per loop then (although this is because of the three separate statements - changing the code to the below will only use one builder per loop):

    Code:
    log += multiplyBy2(i) + add(i, 3) + halve(i);
    Even with this change, a new builder will still be created per iteration.


    Now for our other methods:

    multiplyBy2:
    Code:
    new java/lang/StringBuilder
    dup
    ldc "Multiplying "
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc " by 2."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String;
    Add:
    Code:
    new java/lang/StringBuilder
    dup
    ldc "Adding "
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc " and "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    iload_2
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc "."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String;
    Halve:
    Code:
    new java/lang/StringBuilder
    dup
    ldc "Dividing "
    invokespecial java/lang/StringBuilder/<init>(Ljava/lang/String;)V // Initialise a StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc " by 2."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    invokevirtual java/lang/StringBuilder/toString()Ljava/lang/String;
    Each of these methods is also creating a new StringBuilder - that's 7 new StringBuilders per iteration. So let's change our code:

    Code:
    	public void run() {
    		int i = 10;
    
    		StringBuilder builder = new StringBuilder();
    		while (i < 1_000_000) { // iterate a lot of times
    			multiplyBy2(i, builder);
    			add(i, 3, builder);
    			halve(i, builder);
    		}
    	}
    
    	public void multiplyBy2(int value, StringBuilder builder) {
    		builder.append("Multiplying ").append(value).append(" by 2.");
    	}
    
    	public void add(int a, int b, StringBuilder builder) {
    		builder.append("Adding ").append(a).append(" and ").append(b).append(".");
    	}
    
    	public void halve(int a, StringBuilder builder) {
    		builder.append("Dividing ").append(a).append(" by 2.");
    	}
    Bytecode:

    run():
    Code:
    bipush 10
    istore_1
    new java/lang/StringBuilder
    dup
    invokespecial java/lang/StringBuilder/<init>()V // Initialise the string builder
    astore_2
    goto 21
    aload_0
    iload_1
    aload_2
    invokevirtual Generic/multiplyBy2(ILjava/lang/StringBuilder;)V
    aload_0
    iload_1
    iconst_3
    aload_2
    invokevirtual Generic/add(IILjava/lang/StringBuilder;)V
    aload_0
    iload_1
    aload_2
    invokevirtual Generic/halve(ILjava/lang/StringBuilder;)V
    iload_1
    ldc 1000000
    if_icmplt 8
    multiplyBy2():
    Code:
    aload_2 // Load the second parameter (the string builder)
    ldc "Multiplied "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append to the StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc " by 2."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    pop
    Halve:
    Code:
    aload_2 // Load the second parameter (the string builder)
    ldc "Divided "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append to the StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder; 
    ldc " by 2."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    pop
    add():
    Code:
    aload_3 // Load the third parameter (the string builder)
    ldc "Added "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder; // Append to the StringBuilder
    iload_1
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc " and "
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    iload_2
    invokevirtual java/lang/StringBuilder/append(I)Ljava/lang/StringBuilder;
    ldc "."
    invokevirtual java/lang/StringBuilder/append(Ljava/lang/String;)Ljava/lang/StringBuilder;
    pop
    Success! Only one StringBuilder is created per iteration.

    Now let's benchmark (1,000,000 was changed to 1,000 because otherwise it would've taken forever to get the benchmark)
    Slow method took an average of 47 ms (n = 1000).
    Fast method took an average of 0.6 ms (n = 1000).
    Over 78 times faster!

    However, please do not think I'm saying that it is a good idea to optimise immediately - write good quality code first, and make it fast (if necessary) later. Making good code faster is easy, but making bad code nicer is not.
    Last edited by Major; 11-04-2014 at 10:01 PM.
    Reply With Quote  
     


  7. #7  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000


    What is unit testing?

    Quote Originally Posted by Wikipedia
    Unit testing is a software testing method by which individual units of source code are tested to determine whether they are fit for use.
    The purpose of unit testing is to ensure that small, individual segments of source code perform as expected, and this testing can be done in an automated manner. This can be used to ensure that changes made to the code in the future do not accidentally break it, which is particularly useful for catching edge cases.

    When is unit testing useful or appropriate?

    Unit testing is appropriate for code that:
    • does not use a database or filesystem;
    • can be separated into distinct, independent parts;
    • is typically quick to run.


    If your code does not meet one or more of the above conditions, another form of testing, such as integration testing, may be more appropriate

    What does a unit test look like?

    Eve wants to test a class she has written, which hides a user's password.

    Eve doesn't use a hash function, because she is a terrible person who doesn't follow good security practices. Even worse, she decides to use rot13. Her class looks like the following (with the actual implementation a consequence of how the latin letters are stored in ASCII):

    Code:
    public final class Rot13PasswordEncoder {
    
    	public String encode(String input) {
    		if (input.isEmpty()) {
    			throw new IllegalArgumentException("Cannot encode an empty password!");
    		} else if (input.length() > 30) {
    			return null;
    		}
    
    		return input.codePoints().map(character -> {
    			if (character >= 'a' && character <= 'm' || character >= 'A' && character <= 'M') {
    				return character + 32;
    			} else if (character >= 'n' && character <= 'z' || character >= 'N' && character <= 'Z') {
    				return character - 32;
    			}
    
    			return character;
    		}).collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append).toString();
    	}
    
    }
    She also uses exceptions terribly and doesn't use interfaces at all. Tsk tsk.

    Eve wants to write a test to ensure that the following conditions are always met:
    • The method will always return null if the input is longer than 30 characters
    • The method will always throw an IllegalArgumentException if the input is empty
    • The method will apply the ROT13 algorithm to latin letters only, and otherwise do nothing.


    This is perfect for a unit test, because it does not require any I/O, is entirely independent of other code, and is very quick to run.

    She decides to use JUnit for her testing (which, unlike most of her other choices, is a good idea). The @Test annotation is the core element of JUnit.

    First she creates the class to store her tests in:

    Code:
    public final class Rot13Tests {
    
    }
    And creates the standard template for a unit test method:

    Code:
    	@Test
    	public void empty() {
    
    	}
    Which needs to call the Rot13PasswordEncoder with the appropriate argument (an empty String):

    Code:
    	@Test
    	public void empty() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		encoder.encode("");
    	}
    When she runs this, JUnit says the test failed, even though it did what she wanted - threw an IllegalArgumentException! This is because JUnit considers any sort of exception that is left uncaught to be a failure. So she reads further on the @Test documentation and works out what to change:

    Code:
            @Test(expected =  IllegalArgumentException.class)
    	public void empty() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		encoder.encode("");
    	}
    Success! Now the test will only pass if an IllegalArgumentException is thrown.

    Emboldened by her success, she adds a test too ensure her passwords can't be too long...

    Code:
            @Test(expected =  IllegalArgumentException.class)
    	public void length() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		encoder.encode("ThisPasswordIsLongerThan30Characters");
    	}
    ... except she doesn't want an Exception to be thrown in this case - she wants to make sure that 'null' is returned. Fortunately, JUnit has an Assert utility class that comes in very handy:

    Code:
    	@Test()
    	public void length() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		Assert.assertNull(encoder.encode("ThisPasswordIsLongerThan30Characters"));
    	}
    This throws an Exception if the encoder does not return null (which will therefore fail the test). Eve also finds out that it is standard practice to use static imports for methods in Assert.

    Now Eve only has one method left to write, which should check that the actual algorithm is applied correctly.

    Code:
    	@Test
    	public void algorithm() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    
    		assertEquals("Uryyb, Jbeyq!", encoder.encode("Hello, World!"));
    		assertEquals("npoqrstuvwxyzabcdefghijklm", encoder.encode("acbdefghijklmnopqrstuvwxyz"));
    		assertEquals("NOPQRSTUVWXYZABCDEFGHIJKLM", encoder.encode("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
    		assertEquals("0123456789", "0123456789");
    	}
    Obviously Eve can't test every possible input, so she tests common things: digits, spaces, and other special characters aren't affected, but a-z and A-Z are converted appropriately. Now her final test class looks like:

    Code:
    public final class Rot13Tests {
    
    	@Test
    	public void algorithm() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    
    		assertEquals("Uryyb, Jbeyq!", encoder.encode("Hello, World!"));
    		assertEquals("npoqrstuvwxyzabcdefghijklm", encoder.encode("acbdefghijklmnopqrstuvwxyz"));
    		assertEquals("NOPQRSTUVWXYZABCDEFGHIJKLM", encoder.encode("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
    		assertEquals("0123456789", "0123456789");
    	}
    
    	@Test(expected = IllegalArgumentException.class)
    	public void empty() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		encoder.encode("");
    	}
    
    	@Test()
    	public void length() {
    		Rot13PasswordEncoder encoder = new Rot13PasswordEncoder();
    		assertNull(encoder.encode("ThisPasswordIsLongerThan30Characters"));
    	}
    
    }
    That's a pretty good set of unit tests (for a really crap class).
    Reply With Quote  
     

  8. Thankful user:


  9. #8  


    Major's Avatar
    Join Date
    Jan 2011
    Posts
    2,997
    Thanks given
    1,293
    Thanks received
    3,556
    Rep Power
    5000
    Please post if you feel something needs to be elaborated on, or have any other suggestions or questions.
    Last edited by Major; 09-01-2014 at 03:48 AM.
    Reply With Quote  
     


  10. #9  
    Ex-Staff

    Koy's Avatar
    Join Date
    Oct 2010
    Posts
    1,871
    Thanks given
    1,299
    Thanks received
    910
    Rep Power
    5000
    Holy shit. This is fantastic.
    Reply With Quote  
     

  11. #10  
    Donator


    Join Date
    Mar 2008
    Posts
    1,945
    Thanks given
    118
    Thanks received
    201
    Rep Power
    2104
    I feel like I just got spanked for all that code i've done over the years. Im glad you really opened the door on changing the programming so many are picking up today.

    Thanks Major for this it was a great read and will be applying a lot of your styles to my programming.
    Attached image
    Reply With Quote  
     

Page 1 of 13 12311 ... LastLast

Thread Information
Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)


User Tag List

Similar Threads

  1. Replies: 29
    Last Post: 09-29-2009, 06:25 PM
  2. Replies: 20
    Last Post: 07-31-2009, 11:19 AM
  3. Making a Java File Write Content on Websites
    By Nima304 in forum Application Development
    Replies: 2
    Last Post: 03-09-2009, 05:13 PM
  4. Replies: 4
    Last Post: 01-16-2009, 03:42 AM
  5. [503-PD] Better Theiving.java
    By Ian... in forum Configuration
    Replies: 4
    Last Post: 08-16-2008, 06:04 AM
Posting Permissions
  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •