Uploaded image for project: 'Commons Math'
  1. Commons Math
  2. MATH-1413

Add generics to Frequency

Rank to TopRank to BottomAttach filesAttach ScreenshotBulk Copy AttachmentsBulk Move AttachmentsVotersWatch issueWatchersCreate sub-taskConvert to sub-taskLinkCloneLabelsUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • None
    • 4.0
    • None
    • Patch

    Description

      The Frequency class has no generic types, but creates an internal TreeMap, with keys with the type `Comparator<?>`. Then whenever you add or get values from the TreeMap, it will throw CCE if you pass a key value with a different type from the others stored in the map.

      Frequency f = new Frequency();
      // Okay
      f.addValue("Ola");
      // Key 12, type Integer, does not match String type. CCE, wrapped in a Math exception
      f.addValue(12);
      

      And even a simpler example without Math classes to help outlining the issue.

      TreeMap<Integer, String> map = new TreeMap<>();
      int key = 1;
      map.put(key, "One");
      // Okay
      map.get(100);
      // TreeMap will try to call 1.1#compareTo against each Key it contains, which can result
      // in a CCE here.
      map.get(1.1);
      

      It is not clear how we could compare the key type to the given v value. Checking if the map is not empty, and then checking the type of the first key doesn't sound very elegant (and not sure if it would be a valid solution).

      A better solution could be to add generics to the class, breaking the current contract (i.e. not allowing three types, boxing to longs, etc), but on the bright side, it won't use CCE as a normal workflow, nor need suppress warnings annotations any more.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            kinow Bruno P. Kinoshita
            kinow Bruno P. Kinoshita
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment