Data Structure

[ljiazh3ng]

Handled by 2 persons.

This epic deals with the following:

• [x] Creation of Block class.
• [x] Creation of BlockManager class (keep track of currently opening Block, provide methods to manipulate Blocks).
• [x] Creation of equivalent ...Storage classes to support persistent storage functionality.

[firzanarmani]

Proposed Block model API:

EDIT:

• Cleanup interface
• Fix typo from Name to Title

interface Block {
    Title getTitle();

    Body getBody();

    List<Block> getChildren();

    Optional<Block> getParent(); // Returns Optional<Empty> if `root` block

    void setTitle(Title title);

    void setBody(Body body);

    void setChild(Block child);

    @Override
    boolean equals(Object o); // Return true if this.parent == o.parent && this.title == o.title (pseudocode)

    @Override
    int hashCode(); // Generate hashCode by Object.hash(parent, title)
}

interface Title {
    /*
     * Static method
     * Returns true if `test` is valid
     * Valid title Alphanumeric characters and spaces, should not be blank.
     */
    static boolean isValidTitle(String test);

    @Override
    String toString();

    @Override
    boolean equals(Object o); // Returns true if the this.title == o.title
}

interface Body {
    /*
     * ! Allow anything in body BUT should not be blank.
     *   UI just shows as is. If parsing markdown (v1.3), show `Unable to parse` if 
     *   invalid
     */
    static boolean isBlankBody(String test);

    @Override
    String toString();
}

Proposed BlockManager API:

interface ReadOnlyBlockManager {
    /**
     * Returns an unmodifiable view of the block manager.
     */
    ObservableList<Block> getBlockList();
}

interface BlockManager {
    void setBlockList(List<Block> blocks);

    void resetData(ReadOnlyBlockManager newData)

    boolean hasBlock(Block b);

    void addBlock(Block b);

    void deleteBlock(Block b);

    Block openBlock(Block b);

    void setBlock(Block targetBlock, Block newBlock);

    List<Block> getBlockParents(Block b); // Returns a list of all parents up to root
}

[ljiazh3ng]

interface Block {
  Title title;
  Body body;
  UUID uuid;

  HashMap<UUID, Block> children;

  Block(String title, String body);

  String getTitle();

  String getBody();

  UUID getUuid(); // UUID preferred over generating Objects.hash(title, body) since two blocks of the same hash can be generated in different directories

  boolean setTitle();

  boolean setBody();
}

Looks good, i am thinking we need a parent attribute as will. That way we can traverse back and forth. I would also like to propose that we have a isSameBlock function since we cannot have blocks with the same title with the same parent. This can be done with a getParent and getTitle function, or we could make the isSameBlock Function.

[firzanarmani]

interface Block {
    Title title;
    Body body;
    UUID uuid;

    Block parent; // On creating the child,
    HashMap<UUID, Block> children;

    Block(String title, String body);

    String getTitle();

    String getBody();

    UUID getUuid(); // UUID preferred over generating Objects.hash(title, body) since two blocks of the same hash can be generated in different directories

    boolean setTitle();

    boolean setBody();

    @Override
    boolean equals(Obj o); // Return true if this.parent == o.parent && this.children == o.children (pseudocode)
}

interface Name {
    /*
     * Static method
     * Returns true if `test` is valid
     * Valid title Alphanumeric characters and spaces, should not be blank.
     */
    static boolean isValidTitle(String test);

    @Override
    String toString();

    @Override
    boolean equals();
}

interface Body {
    /*
     * ! No need for validity check. Allow anything in body BUT should not be blank.
     *   UI just shows as is. If parsing markdown (v1.3), show `Unable to parse` if 
     *   invalid
     */
    static boolean isBlankBody(String test);

    @Override
    String toString();
}

@HemanshuGandhi @ljiazh3ng @kevinputera @johannagwan What do y'all think?

[kevinputera]

Hey @firzanarmani thanks for this!

First things first, I don't think you need to show us the fields in these interfaces! This also eliminates any possibility of us misusing your implementation details! Only methods (parameters + return value) would suffice.

interface Block {
    Title title;
    Body body;
    UUID uuid;

    Block parent; // On creating the child,
    HashMap<UUID, Block> children;

    Block(String title, String body);

    String getTitle();

    String getBody();

    UUID getUuid(); // UUID preferred over generating Objects.hash(title, body) since two blocks of the same hash can be generated in different directories

    boolean setTitle();

    boolean setBody();

    @Override
    boolean equals(Obj o); // Return true if this.parent == o.parent && this.children == o.children (pseudocode)
}

• Why do we need the getUuid() method? I believe that every block at a certain level should be uniquely identified by its title right?
• I believe we need a List<Block> getChildren() method, what do you think?
• In addition, how about providing an Optional<Block> getParent() method?
• Why do we return boolean values for setBody() and setTitle? Also, let's be specific about the parameters for both boolean setBody(Body body) and boolean setTitle(Title title).
• Lastly, shouldn't getBody() returns Body, while getTitle() returns Title?

interface Name {
    /*
     * Static method
     * Returns true if `test` is valid
     * Valid title Alphanumeric characters and spaces, should not be blank.
     */
    static boolean isValidTitle(String test);

    @Override
    String toString();

    @Override
    boolean equals();
}

• Not sure if it's a typo, but Name should be Title instead right?
• Should we provide a String getText() method that returns the text content of the Title?

interface Body {
    /*
     * ! No need for validity check. Allow anything in body BUT should not be blank.
     *   UI just shows as is. If parsing markdown (v1.3), show `Unable to parse` if 
     *   invalid
     */
    static boolean isBlankBody(String test);

    @Override
    String toString();
}

• Should we provide a String getText() method that returns the text content of the Body?

Everything looks good so far, but I think we're missing something here.

1. We need a model that keeps track of the currently opened Block (basically the block that is active and shown on the screen). Think of it as the Finder in Mac OS, while the current Block class is more like the individual folders.
2. In addition, we need to think of a way such that any changes to a particular block get propagated to the UI (most likely through the usage of JavaFX's Property interface). The concern is that if we call one of the setters of the Block class directly, the change won't be detected from the UI layer. Any ideas on how we can do this?

[firzanarmani]

I would also like to propose that we have a isSameBlock function since we cannot have blocks with the same title with the same parent. This can be done with a getParent and getTitle function, or we could make the isSameBlock Function.

@ljiazh3ng Would the edited equals() in the Block model be sufficient? Or would it be better to write out an isSameBlock()?

@kevinputera Apologies! I wasn't careful in pasting the correct Block class! I've also removed the fields as you suggested and fixed the typo to Title. Thanks for pointing that out. I've made the changes in an edit to the first reply to clean up the thread.

• Should we provide a String getText() method that returns the text content of the Title?

I feel that using an overridden toString() method should suffice for both Title and Body. What do you think?

1. We need a model that keeps track of the currently opened Block (basically the block that is active and shown on the screen). Think of it as the Finder in Mac OS, while the current Block class is more like the individual folders.

I was thinking of keeping track of the current path in the BlockManager. I've just added a proposed API for the BlockManager. Let me know if you think this is okay or if you have anything to suggest! @ljiazh3ng @kevinputera

[kevinputera]

@kevinputera Apologies! I wasn't careful in pasting the correct Block class! I've also removed the fields as you suggested and fixed the typo to Title. Thanks for pointing that out. I've made the changes in an edit to the first reply to clean up the thread.

• Should we provide a String getText() method that returns the text content of the Title?

I feel that using an overridden toString() method should suffice for both Title and Body. What do you think?

1. We need a model that keeps track of the currently opened Block (basically the block that is active and shown on the screen). Think of it as the Finder in Mac OS, while the current Block class is more like the individual folders.

I was thinking of keeping track of the current path in the BlockManager. I've just added a proposed API for the BlockManager. Let me know if you think this is okay or if you have anything to suggest! @ljiazh3ng @kevinputera

@firzanarmani No worries, thanks a lot! Please give me a while, I'll consolidate my thoughts and reply by tonight

[kevinputera]

Hey @firzanarmani , sorry for the wait!

---

I feel that using an overridden toString() method should suffice for both Title and Body. What do you think?

Regarding this, I actually am leaning towards adding a getText() method. It's not really obvious that the toString() method returns only the text property of both the Title and Body (although it's implicitly true in this case). In fact, usually the toString() method will be implemented to return something along the lines of "ClassName(property=value)" (this is how IDEs generate toString() methods I believe).

Providing the getText() method just makes it explicit that the method will only return the text property, and will never change whether or not we add more fields to the Title and Body classes in the future!

---

I was thinking of keeping track of the current path in the BlockManager. I've just added a proposed API for the BlockManager. Let me know if you think this is okay or if you have anything to suggest!

Yes, I believe adding this BlockManager class is the way to go! I am not sure what's the difference between a ReadOnlyBlockManager and a BlockManager, though? IIRC, the relationship between ReadOnlyAddressBook and AddressBook in AB3 lies in that ReadOnlyAddressBook is the interface, while AddressBook is the concrete implementation.

---

interface ReadOnlyBlockManager {
    /**
     * Returns an unmodifiable view of the block manager.
     */
    ObservableList<Block> getBlockList();
}

• How does our tree-like structure, i.e. a recursive tree of Blocks, map to an ObservableList<Block>?
• I was thinking that, instead of providing a list of Blocks, we provide a reference to the currently opened Block. Whoever uses this model can then traverse through the whole tree through the getParent() and getChildren() of that opened Block. We can provide the reference to this Block via JavaFX's Property class, which has observability behavior. That is, we provide an API along the lines of Property<Block> getCurrentlyOpenedBlock()

interface BlockManager {
    void setBlockList(List<Block> blocks);

    void resetData(ReadOnlyBlockManager newData)

    boolean hasBlock(Block b);

    void addBlock(Block b);

    void deleteBlock(Block b);

    Block openBlock(Block b);

    void setBlock(Block targetBlock, Block newBlock);

    List<Block> getBlockParents(Block b); // Returns a list of all parents up to root
}

• Similar to above, how does our Block tree map to a list of Blocks?
• Why do we need the hasBlock() and getBlockParents() method?
• In addition, how does setBlock() work? I'm asking this because the Block class is already mutable. That is, you can call one of the setters of the Block class to update the individual Blocks (note, however, that this kind of updating won't be propagated to the UI when we use JavaFX's Observables). I think we do need this setBlock() method, but perhaps we should make the Block class immutable instead? This is so that the consumer of your API only have one way to update the Block (through the BlockManager instance) and reduce misuse.
• Lastly, I believe we should make the parameters of the hasBlock(), addBlock(), deleteBlock(), openBlock() methods accept a special Path object (@ljiazh3ng please confirm! I'm thinking of letting the Parser class parse the raw string into this Path object which then get passed into this BlockManager class), instead of the direct Block. This is because the users of these APIs won't have a reference to the Block they want to open, delete, etc. in the first place.

[firzanarmani]

It's cool @kevinputera. Thanks for the comments!

It's not really obvious that the toString() method returns only the text property of both the Title and Body (although it's implicitly true in this case).

I see! Alright, I'm agreement! I'll add this.

---

Yes, I believe adding this BlockManager class is the way to go! I am not sure what's the difference between a ReadOnlyBlockManager and a BlockManager, though? IIRC, the relationship between ReadOnlyAddressBook and AddressBook in AB3 lies in that ReadOnlyAddressBook is the interface, while AddressBook is the concrete implementation.

Yeah, you're right. I probably misunderstood it. So, I will remove the ReadOnlyBlockManager and move the getBlockList() to BlockManager instead.

---

Why do we need the hasBlock() and getBlockParents() method?

My actual intention was actually pointed out in your last point:

Lastly, I believe we should make the parameters of the hasBlock(), addBlock(), deleteBlock(), openBlock() methods accept a special Path object (@ljiazh3ng please confirm! I'm thinking of letting the Parser class parse the raw string into this Path object which then get passed into this BlockManager class), instead of the direct Block. This is because the users of these APIs won't have a reference to the Block they want to open, delete, etc. in the first place.

However, we haven't made any decision regarding how to parse the Block's path for the methods mentioned above. Will discuss again.

---

That is, you can call one of the setters of the Block class to update the individual Blocks (note, however, that this kind of updating won't be propagated to the UI when we use JavaFX's Observables). I think we do need this setBlock() method, but perhaps we should make the Block class immutable instead?

With the Observables in mind, I understand the need for Block class to be immutable. Will discuss first before making changes again!

With all that being said, let's meet up and discuss this again.

[ljiazh3ng]

Proposed API:

interface Path {
    public String[] getPath();
}

As mentioned in the team meeting today. The openBlock command can take in a Path object and open nested blocks. The deleteBlock command can also take in a Path object to delete a block. I don't think it would be necessary for hasBlock to take in a path for now as it would only be used to prevent duplicate block("Same title and same parent") on the current directory.

[ljiazh3ng]

Proposed BlockManager API:

interface BlockManager {
    void setBlockList(List<Block> blocks);

    ObservableList<Block> getBlockList();

    boolean hasBlock(Block b);

    void addBlock(Block b);

    void deleteBlock(Path p);

    Block openBlock(Path p);

    void setBlock(Block targetBlock, Block newBlock)

    List<Block> getBlockParents(Block b); // Returns a list of all parents up to root
}

Propose to change Delete and OpenBlock function to take in the parameter Path object instead.