Review-Request: UnsureType::addType for unique-lists

[Niko Sams]

On Mon, May 11, 2009 at 5:34 PM, Milian Wolff <mail at milianw.de> wrote:
> Hey guys!
>
> At least for return-types it doesn't make much sense imo to add the same
>  type multiple times to the UnsureType, example:
>
> ~~~
> function foo() {
>  if ( ... ) {
>    return true;
>  } else if ( ... ) {
>    return false;
>  }
>
>  // default
>  return null;
> }
> ~~~
>
> This should be: UnsureType(Bool, Null);
>
> Right now in PHP it would be (Bool, Bool, Null);
>
> I could change PHP code to check whether the type is already set, but the
> easiest (and least code) would be to change the addType function to the
> following:
>
> ~~~
>
> Index: language/duchain/types/unsuretype.cpp
> ===================================================================
> --- language/duchain/types/unsuretype.cpp       (Revision 966510)
> +++ language/duchain/types/unsuretype.cpp       (Arbeitskopie)
> @@ -98,8 +98,10 @@
>   KDevelop::AbstractType::exchangeTypes(exchanger);
>  }
>
> -void UnsureType::addType(KDevelop::IndexedType type) {
> -  d_func_dynamic()->m_typesList().append(type);
> +void UnsureType::addType(KDevelop::IndexedType type, bool alwaysAdd) {
> +  if ( alwaysAdd || !d_func_dynamic()->m_typesList().contains(type) ) {
> +    d_func_dynamic()->m_typesList().append(type);
> +  }
>  }
>
>  void UnsureType::removeType(KDevelop::IndexedType type) {
> Index: language/duchain/types/unsuretype.h
> ===================================================================
> --- language/duchain/types/unsuretype.h (Revision 966510)
> +++ language/duchain/types/unsuretype.h (Arbeitskopie)
> @@ -62,7 +62,9 @@
>   virtual KDevelop::AbstractType::WhichType whichType() const;
>   virtual void exchangeTypes(KDevelop::TypeExchanger* exchanger);
>
> -  void addType(IndexedType type);
> +  /// Add a type to the list
> +  /// @p alwaysAdd set to false if you don't want to add the same type
> multiple times
> +  void addType(KDevelop::IndexedType type, bool alwaysAdd = true);
>   void removeType(IndexedType type);
>
>   ///Array of represented types. You can conveniently iterate it using the
> FOREACH_FUNCTION macro,
>
> ~~~
>
> Comments? Can I commit? Should it not be a param but another method?
It makes never sense to have the same type twice in an UnsureType.
So I think this should not even be a parameter.
But still david should review this.

If we add such convenience functionality we should also think about
when adding another UnsureType those two should be merged.

Niko