Adding a real feature to a project 🧛

While looking for an issue for my 3rd hacktoberfest pull request, I stumbled accross a feature request, that looked fun and interesting to work on. The project was a classic quiz application, simulating practice exams for azure fundamentals.

If you visit the link and try to click on Exam Mode, you'll get a placeholder screen as the all those changes still need to be shipped to production.

The maintainer is only adding features to dev branch first, which is why I would be using localhost on dev to demonstrate in the blog.

Table of Contents

 1. The Issue
 2. Implementing the Feature 🛠️
       2.1. Looking for a queue implementation
       2.2. Queue Implementation 🚶🚶🚶🚶
 3. The Pull Request 🧲
       3.3. Additional Commits
 4. Conclusion 🎊

The Issue

Before talking about the issue, lets take a quick look at how the application worked at the time.

On clicking Begin Exam, a timer started and you could either click Next or Skip button after selecting a response.

In its the then state, the Skip button behaved same as the Next button. The issue opened by maintainer suggested that it was a bug that needed to be fixed.

However, I would say that was more of a brand-new feature request as not a single part of the logic required was implemented yet.

Here is the expected behavior in the owner's words.

Now it was time to get to work!

Implementing the Feature 🛠️

The very first step towards achieving this goal was to figure out what kind of queue I would use. I started looking for a reliable package that implemented the queue data structure for me.

Looking for a queue implementation

The very first one I came across was a package called useHooks. It looked like a really cool package with tonnes of hooks to use, and useQueue was one of them.

I quickly installed and tested the hook in my local environment, and it worked like a charm. However, adding such a heavy package just for using one single hook didn't make sense to me.

There had to be a better way!

I spent the next hour searching for an alternative package that specifically focussed on that data structure, and I was surprised to see there is neither a built-in queue, nor a de-facto standard library/package for this purpose in js.

I found a few packages like queue, but they are more like message queues, to collect and execute functions/actions in order.

There were some packages like queue-fifo which solved my purpose, but had low popularity (weekly downloads), and last published 4 years ago. We cannot possibly trust such a package, as it would go against the principles of Software Supply Chain Security.

Queue Implementation 🚶🚶🚶🚶

After all this research on various platforms, I decided I'll have to roll my own queue. So I created a new directory called utils and started writing my own implementation in a Queue.ts file. I tried making a generic class so the user could specify the type they want to store in the queue.

Here's what my implementation looked like:

/**
 * A data structure implementing FIFO (first in, first out) behavior
 */
export class Queue<T> {
    private collection: T[];

    constructor(initialCollection: T[] = []) {
        this.collection = initialCollection;
    }

    /**
     * Adds an element to the back of the queue
     * 
     * @param {T} element The element to be added
     */
    enqueue(element: T) {
        this.collection.push(element);
    }

    /**
     * Removes and returns the element from the front of the queue
     * 
     * @returns The removed element
     */
    dequeue(): T | undefined {
        return this.collection.shift();
    }

    /**
     * 
     * @returns The element at the front of the queue if present, else null
     */
    front(): T | null {
        return this.collection[0] ?? null;
    }

    /**
     * 
     * @returns Total number of elements in the queue
     */
    size(): number {
        return this.collection.length;
    }

    /**
     * 
     * @returns True if the queue is empty
     */
    isEmpty(): boolean {
        return this.size() === 0;
    }

    /**
     * Removes all the elements from the queue
     */
    clear(): void {
        while (!this.isEmpty()) {
            this.dequeue();
        }
    }

    /**
     * 
     * @returns Returns an array with all the elements in the queue
     */
    queue(): T[] {
        return [...this.collection];
    }
}

Once I was satisfied that my queue behaved as expected, I started working on the feature itself.

This is the workflow I decided:

• Store the skipped question indexes in a Queue using component state.

const [skippedQuestions, setSkippedQuestions] = useState<Queue<number>>(
    new Queue<number>(),
  );

• Whenever Skip button gets clicked, add the question indexes to the queue, delete any selected response from the responseSet, and invoke the handleNextQuestion function.

const handleSkipQuestion = (questionNo: number) => {
    skippedQuestions.enqueue(questionNo);

    // Unset any selected answer
    setAnswers((prevState) => {
      const updatedAnswers = { ...prevState };

      // Reset the response if an answer was selected
      if (updatedAnswers.hasOwnProperty(questionNo)) {
        delete updatedAnswers[questionNo];
      }

      // Replace with the updated state
      return updatedAnswers;
    });

    handleNextQuestion(questionNo + 1);
  };

• Update the handleNextQuestion function to move to next question if all questions haven't been touched yet. If all questions have been touched, move to the next skipped question in the queue, if any, else invoke the checkPass function to end the test and display results.

const [allQuestionsTouched, setAllQuestionsTouched] =
    useState<boolean>(false);

const handleNextQuestion = (questionNo: number) => {
    if (questionNo <= numberOfQuestions) {
      if (!allQuestionsTouched) {
        setCurrentQuestionIndex(questionNo);
      } else if (!skippedQuestions.isEmpty()) {
        setCurrentQuestionIndex(
          skippedQuestions.dequeue() ?? numberOfQuestions,
        );
      } else {
        setCurrentQuestionIndex(numberOfQuestions);
        checkPassed();
      }
    } else {
      setAllQuestionsTouched(true);

      if (!skippedQuestions.isEmpty()) {
        setCurrentQuestionIndex(
          skippedQuestions.dequeue() ?? numberOfQuestions,
        );
      } else {
        setCurrentQuestionIndex(numberOfQuestions);
        checkPassed();
      }
    }
  };

And added a fix to keep the Next button disabled until a response was selected:

After I finished my logic and everything worked as expected, it was time to open the Pull Request.

The Pull Request 🧲

I was expecting that this would be a smooth merge as well, but to my surprise, this was the longest conversation I ever had in an open source contribution, 29 conversations long.

You can take a look yourself:
https://github.com/eduardconstantin/azure-fundamentals/pull/82

Additional Commits

The very first changes maintainer requested were to convert the Queue class to a function (probably because the project was using functional approach), and reported a bug where the skipped question, had the option selected that user might have clicked before skipping. To be honest, I felt like that was more like a feature than a bug, but it wasn't my project so...

The maintainer gave me an option to either fix the bug, or open another issue for it, but I decided to take it on.

The first thing I did was update my Queue class to a factory function. After lots of research and revision of the syntax, this was the end result:

export interface IQueue<T> {
    enqueue(element: T): void;
    dequeue(): T | undefined;
    front(): T | null;
    size(): number;
    isEmpty(): boolean;
    clear(): void;
    queue(): T[];
}

/**
 * A data structure implementing FIFO (first in, first out) behavior
 */
export function Queue<T>(initialCollection: T[] = []): IQueue<T> {
    // Private collection
    let collection: T[];

    function initQueue(): IQueue<T> {
        collection = initialCollection;

        return {
            enqueue,
            dequeue,
            clear,
            front,
            queue,
            isEmpty,
            size
        }
    }

    initQueue();

    /**
     * Adds an element to the back of the queue
     * 
     * @param {T} element The element to be added
     */
    function enqueue(element: T) {
        collection.push(element);
    }

    /**
     * Removes and returns the element from the front of the queue
     * 
     * @returns The removed element
     */
    function dequeue(): T | undefined {
        return collection.shift();
    }

    /**
     * 
     * @returns The element at the front of the queue if present, else null
     */
    function front(): T | null {
        return collection[0] ?? null;
    }

    /**
     * 
     * @returns Total number of elements in the queue
     */
    function size(): number {
        return collection.length;
    }

    /**
     * 
     * @returns True if the queue is empty
     */
    function isEmpty(): boolean {
        return size() === 0;
    }

    /**
     * Removes all the elements from the queue
     */
    function clear(): void {
        while (!isEmpty()) {
            dequeue();
        }
    }

    /**
     * 
     * @returns Returns an array with all the elements in the queue
     */
    function queue(): T[] {
        return [...collection];
    }

    return initQueue();
}

I know IQueue is a bad naming practive for an interface, but I couldn't name it Queue due to naming conflicts. Please let me know what would be a better name for it if you've made it till here 😉.

These were the new commits in response to the requested changes.

You'll notice I also had to fix merge conflicts as there was already progress on the dev branch.

Now that I thought everything was good to be merged, another fellow contributor joined the PR as a reviewer and suggested a couple of changes.

First, I wasn't updating the answers state properly.

Second, I forgot to setup husky to format my code through commits.

I quickly fixed those problems and pushed a new commit.

You'll notice there's a force push. That was because I amended my commit message.

But that was not it, the converstation went on and on. It was a bug discussion, some requirements were changed, some issues were found and everything was addressed, and the Pull Request was finally mereged .

Its better to follow the actual converstation here.

Conclusion 🎊

This was, by far, the largest open source contribution I had ever made.

There was a comprehensive discussion, with more than 2 people involved, and about 10 commits until the PR was merged.

Lots of learnings, and even though I got frustrated many times, all that work payed off in the end. In other words, this PR taught me the importance of patience while contributing to open source projects.