[webkit-dev] Proposal: WTF HashMap iterators to use key/value instead first/second

[Maciej Stachowiak]

I like the idea of this change. Using key and value makes things more readable at the site of use than a generic first/second. It's unfortunate that it is hard to deploy, but I think it's worth it to work through those challenges.	`

-Maciej

On Aug 28, 2012, at 3:24 PM, Caio Marcelo de Oliveira Filho <caio.oliveira at openbossa.org> wrote:

> Hello,
> 
> I would like to propose we change HashMap iterators to use 'key' and
> 'value' instead of 'first' and 'second'. The work is being tracked in
> the bug https://bugs.webkit.org/show_bug.cgi?id=82784.
> 
> The idea came from https://bugs.webkit.org/show_bug.cgi?id=71063#c2. I
> argue that this change will make code more readable, specially in
> situations where either the key or the value is a pair. One example of
> this situation is in
> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
> 
>             GetterSetterMap::AddResult result =
> map.add(node->name().impl(), pair);
>             if (!result.isNewEntry)
> -                result.iterator->second.second = node;
> +                result.iterator->value.second = node;
> 
> another one is in
> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp
> 
>     for (CCLayerTilingData::TileMap::const_iterator iter =
> m_tiler->tiles().begin(); iter != m_tiler->tiles().end(); ++iter) {
> -        int i = iter->first.first;
> -        int j = iter->first.second;
> -        UpdatableTile* tile = static_cast<UpdatableTile*>(iter->second.get());
> +        int i = iter->key.first;
> +        int j = iter->key.second;
> +        UpdatableTile* tile = static_cast<UpdatableTile*>(iter->value.get());
> 
> I also think that in "non-pair key and non-pair value" situations the
> readability is improved.
> 
> One downside of this change is that STL uses first and second in its
> maps so that would confuse people. After inspecting the whole
> repository for 'first' and 'second', the usage of std::map in WebKit
> is an exception not a rule, IMHO this makes the confusion less
> troublesome.
> 
> Are there other downsides am I missing? What do you guys think? Unless
> we figure this is a bad change for the project, I intend to land it.
> 
> I've attempted to land this previously, I found out it caused a major
> breakage in some internal builds. I apologize for that inconvenience
> and the time lost.
> 
> I'm looking into ways to make the transition smoother, but until now
> couldn't find a solution that doesn't require C++11. Either way, if we
> land this change, I'll sync with developers from the affected ports
> that I know could be internally affected to avoid more breakage.
> 
> 
> Best regards,
> 
> -- 
> Caio Marcelo de Oliveira Filho
> openBossa @ INdT - Instituto Nokia de Tecnologia
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev