[Java] best practices
От: blackhearted Украина  
Дата: 26.04.11 14:53
Оценка: -1
пример кода...

public List<BandInfo> getBandInfos()
{
    List list;
    synchronized (this.m_bandList) {
        list = new LinkedList(this.m_bandList);
    }
    return list;
}


продакшн.
Re: [Java] best practices
От: Muxa  
Дата: 26.04.11 15:28
Оценка: +4
B>
так, и чо?
(для неписавшего на java)
Re: [Java] best practices
От: UDI Россия  
Дата: 26.04.11 18:35
Оценка:
Здравствуйте, blackhearted, Вы писали:

B>пример кода...


B>
B>public List<BandInfo> getBandInfos()
B>{
B>    List list;
B>    synchronized (this.m_bandList) {
B>        list = new LinkedList(this.m_bandList);
B>    }
B>    return list;
B>}
B>


B>продакшн.


Могу предположить, что m_bandList это обернутая Collections.synchrnozied<...>(Collection) коллекция. Тогда вроде все правильно, возвращается копия этой коллекции для безопасной работы с ней в вызывающем потоке (но все действия над ней не отразятся на m_bandList), причем эта копия может быть разная у потоков, вызвавших этот метод хоть даже почти в одно и тоже время (например, если при этом будет меняться m_bandList).


public static <T> Collection<T> synchronizedCollection(Collection<T> c)
Returns a synchronized (thread-safe) collection backed by the specified collection. In order to guarantee serial access, it is critical that all access to the backing collection is accomplished through the returned collection.

It is imperative that the user manually synchronize on the returned collection when iterating over it:
Collection c = Collections.synchronizedCollection(myCollection);
...
synchronized(c) {
Iterator i = c.iterator(); // Must be in the synchronized block
while (i.hasNext())
foo(i.next());
}

Failure to follow this advice may result in non-deterministic behavior.

"Не волнуйся, голова! Теперь будет думать компьютер"
Гомер Джей Симпсон
Re: [Java] best practices
От: Eugeny__ Украина  
Дата: 26.04.11 19:44
Оценка: 1 (1) +1
Здравствуйте, blackhearted, Вы писали:

B>пример кода...


B>
B>public List<BandInfo> getBandInfos()
B>{
B>    List list;
B>    synchronized (this.m_bandList) {
B>        list = new LinkedList(this.m_bandList);
B>    }
B>    return list;
B>}
B>


B>продакшн.


Я, наверное, говнокодер. Но без контекста не вижу ничего страшного(разве что на кой черт LinkedList, но опять же, задачи разные, а из этого куска нифига непонятно). Если m_bandList это просто List<BandInfo>, и нужно возвратить новый инстанс коллекции, не изменяющий оригинальную(а это логично, учитывая, что геттер называется не как поле, т.е. данный метод это не геттер по сути), то как это делать по-другому? Ну еще коробит название поля, но это уже зависит от стиля в данной компании, опять же, без объяснений это не говнокод.
Новости очень смешные. Зря вы не смотрите. Как будто за наркоманами подсматриваешь. Только тетка с погодой в завязке.
There is no such thing as a winnable war.
Re[2]: [Java] best practices
От: UDI Россия  
Дата: 26.04.11 20:29
Оценка:
Здравствуйте, Eugeny__, Вы писали:

E__>Я, наверное, говнокодер. Но без контекста не вижу ничего страшного(разве что на кой черт LinkedList, но опять же, задачи разные, а из этого куска нифига непонятно). Если m_bandList это просто List<BandInfo>, и нужно возвратить новый инстанс коллекции, не изменяющий оригинальную(а это логично, учитывая, что геттер называется не как поле, т.е. данный метод это не геттер по сути), то как это делать по-другому? Ну еще коробит название поля, но это уже зависит от стиля в данной компании, опять же, без объяснений это не говнокод.


LinkedList, видимо, для того чтобы как можно быстрее создать копию m_bandList, так как в это время модификация m_bandList (я предполагаю, что это synchronized обертка над коллекцией) блокируется. И поэтому немаловажно, что все это в synchronized блоке, без него получилось бы фиг знает что. Думаю, назначение метода — потокобезопасное создание копии коллекции m_bandList на текущий момент времени.
"Не волнуйся, голова! Теперь будет думать компьютер"
Гомер Джей Симпсон
Re: [Java] best practices
От: gegMOPO4  
Дата: 07.05.11 09:26
Оценка:
Здравствуйте, blackhearted, Вы писали:
B>продакшн.

Что смущает?
Re: [Java] best practices
От: lollipop  
Дата: 07.05.11 11:59
Оценка:
Здравствуйте, blackhearted, Вы писали:

B>пример кода...


B>
B>public List<BandInfo> getBandInfos()
B>{
B>    List list;
B>    synchronized (this.m_bandList) {
B>        list = new LinkedList(this.m_bandList);
B>    }
B>    return list;
B>}
B>


B>продакшн.

Хм чёто непойму это потокобезопасный код? Переменная list вроде как нет. А как ломается? возвращается одинаковое значение для нескольких потоков?
Re[3]: [Java] best practices
От: vsb Казахстан  
Дата: 10.05.11 10:04
Оценка:
Здравствуйте, UDI, Вы писали:

UDI>Здравствуйте, Eugeny__, Вы писали:


E__>>Я, наверное, говнокодер. Но без контекста не вижу ничего страшного(разве что на кой черт LinkedList, но опять же, задачи разные, а из этого куска нифига непонятно). Если m_bandList это просто List<BandInfo>, и нужно возвратить новый инстанс коллекции, не изменяющий оригинальную(а это логично, учитывая, что геттер называется не как поле, т.е. данный метод это не геттер по сути), то как это делать по-другому? Ну еще коробит название поля, но это уже зависит от стиля в данной компании, опять же, без объяснений это не говнокод.


UDI>LinkedList, видимо, для того чтобы как можно быстрее создать копию m_bandList, так как в это время модификация m_bandList (я предполагаю, что это synchronized обертка над коллекцией) блокируется. И поэтому немаловажно, что все это в synchronized блоке, без него получилось бы фиг знает что. Думаю, назначение метода — потокобезопасное создание копии коллекции m_bandList на текущий момент времени.


ArrayList создать быстрее. Помимо нарушения Sun Coding Conventions в названии переменной, криминального в коде ничего не вижу, хотелось бы пояснений от топикстартера.
Re: [Java] best practices
От: MxMsk Португалия  
Дата: 10.05.11 10:14
Оценка: 1 (1)
Здравствуйте, blackhearted, Вы писали:

B>пример кода...


B>
B>public List<BandInfo> getBandInfos()
B>{
B>    List list;
B>    synchronized (this.m_bandList) {
B>        list = new LinkedList(this.m_bandList);
B>    }
B>    return list;
B>}
B>


B>продакшн.


Я думаю, проблема в том, что наружу выдается модифицируемая коллекция, которая по факту ничего толкового модифицировать не сможет. Вызывая getBandInfos и ожидая на выходе изменяемый список, вполне логично решить, что мы может влиять на список групп. Но это иллюзия, потому что а) — список list никак не связан с m_bandList, б) — список m_bandList требует синхронизации доступа, о которой явно нигде не сообщается. Таким образом, корректнее было бы нечто вроде:
public IEnumerable<BandInfo> getBandInfos()
{
    List list;
    synchronized (this.m_bandList) {
        list = new LinkedList(this.m_bandList);
    }
    return list;
}

по C#-ному.
Re[2]: [Java] best practices
От: neFormal Россия  
Дата: 11.05.11 11:22
Оценка:
Здравствуйте, Muxa, Вы писали:

B>>

M>так, и чо?
M>(для неписавшего на java)

может это такая реализация const ?.
...coding for chaos...
Re[4]: [Java] best practices
От: Eugeny__ Украина  
Дата: 12.05.11 17:27
Оценка:
Здравствуйте, vsb, Вы писали:


UDI>>LinkedList, видимо, для того чтобы как можно быстрее создать копию m_bandList, так как в это время модификация m_bandList (я предполагаю, что это synchronized обертка над коллекцией) блокируется. И поэтому немаловажно, что все это в synchronized блоке, без него получилось бы фиг знает что. Думаю, назначение метода — потокобезопасное создание копии коллекции m_bandList на текущий момент времени.


vsb>ArrayList создать быстрее.


Не, создается таки линкованный быстрее. Напиши тестик для большого количества данных — там в разы разница. Но это, опять же, ни о чем не говорит. Мы даже примерно не знаем даже какого размера коллекции предполагаются, и как они будут использоваться. Тот же линкед создать-то быстрее, но если к нему планируется пару обращений, то не беда. Если по нему будет много рандомных обращений по индексу... Ну вы поняли.

Присоединяюсь к остальным в ожидании от Тс, что же в коде не так. Без контекста код как код.
Новости очень смешные. Зря вы не смотрите. Как будто за наркоманами подсматриваешь. Только тетка с погодой в завязке.
There is no such thing as a winnable war.
Re[5]: [Java] best practices
От: gegMOPO4  
Дата: 12.05.11 18:47
Оценка:
Здравствуйте, Eugeny__, Вы писали:
E__>Здравствуйте, vsb, Вы писали:
vsb>>ArrayList создать быстрее.
E__>Не, создается таки линкованный быстрее. Напиши тестик для большого количества данных — там в разы разница.

А давайте напишем. Я так был ошарашен таким парадоксальным заявлением, что написал.
import java.util.*;
public class testCreateList
{
    public static void main( String[] args )
    {
        final int N = Integer.parseInt( args[0] );
        Integer[] data = new Integer[N];
        for( int i = 0; i < data.length; ++i )
            data[i] = i;
        List<Integer> src = Arrays.asList( data );
        for( int i = 0; i < 10; ++i )
        {
            long t = System.nanoTime();
            new ArrayList<Integer>( src );
            System.err.print( System.nanoTime() - t );
            System.err.print( '\t' );
            t = System.nanoTime();
            new LinkedList<Integer>( src );
            System.err.println( System.nanoTime() - t );
        }
    }
}


Действительно, повторное создание LinkedList малого размера (меньше 5 элементов) может быть незначительно быстрее, чем ArrayList. Для больших же списков LinkedList отстаёт, как и ожидалось, на порядок. Ну и первое создание — LinkedList тормозит страшно всегда.

Ну а рекордсмен по скорости: Arrays.asList( src.toArray( new Integer[src.size()] ) ).
Re[6]: [Java] best practices
От: Eugeny__ Украина  
Дата: 12.05.11 19:08
Оценка:
Здравствуйте, gegMOPO4, Вы писали:


vsb>>>ArrayList создать быстрее.

E__>>Не, создается таки линкованный быстрее. Напиши тестик для большого количества данных — там в разы разница.

MOP>А давайте напишем. Я так был ошарашен таким парадоксальным заявлением, что написал.

MOP>
MOP>import java.util.*;
MOP>public class testCreateList
MOP>{
MOP>    public static void main( String[] args )
MOP>    {
MOP>        final int N = Integer.parseInt( args[0] );
MOP>        Integer[] data = new Integer[N];
MOP>        for( int i = 0; i < data.length; ++i )
MOP>            data[i] = i;
MOP>        List<Integer> src = Arrays.asList( data );
MOP>        for( int i = 0; i < 10; ++i )
MOP>        {
MOP>            long t = System.nanoTime();
MOP>            new ArrayList<Integer>( src );
MOP>            System.err.print( System.nanoTime() - t );
MOP>            System.err.print( '\t' );
MOP>            t = System.nanoTime();
MOP>            new LinkedList<Integer>( src );
MOP>            System.err.println( System.nanoTime() - t );
MOP>        }
MOP>    }
MOP>}
MOP>


MOP>Действительно, повторное создание LinkedList малого размера (меньше 5 элементов) может быть незначительно быстрее, чем ArrayList. Для больших же списков LinkedList отстаёт, как и ожидалось, на порядок. Ну и первое создание — LinkedList тормозит страшно всегда.


MOP>Ну а рекордсмен по скорости: Arrays.asList( src.toArray( new Integer[src.size()] ) ).


Последнее бесспорно. Но у меня на работе есть тест, показывающий прямо обратное, но тестирующий более похожий на код ТС действия. И в совсем других условиях. Завтра напишу — влом дома все ставить и воспроизводить.

ЗЫ в реале такие тесты(и мой, и ваш) имеют значение только если это жутко узкое место, и нужна оптимизация именно тут. Я же не зря в самом начале выразил удивление использованием линкедлиста в этом коде(но мало ли, может правда надо, из этого куска нифига неясно, и контракт этого метода даже близко неизвестен). Топикстартер врядли именно это имел ввиду, постя этот код. Полагаю, что что-то не понравилось ему как сишнику, но он молчит как партизан.
Новости очень смешные. Зря вы не смотрите. Как будто за наркоманами подсматриваешь. Только тетка с погодой в завязке.
There is no such thing as a winnable war.
Re[7]: [Java] best practices
От: gegMOPO4  
Дата: 12.05.11 19:45
Оценка:
Здравствуйте, Eugeny__, Вы писали:
E__>Последнее бесспорно. Но у меня на работе есть тест, показывающий прямо обратное, но тестирующий более похожий на код ТС действия. И в совсем других условиях. Завтра напишу — влом дома все ставить и воспроизводить.

Если средний размер списка будет больше нескольких элементов, буду удивлён.

E__>ЗЫ в реале такие тесты(и мой, и ваш) имеют значение только если это жутко узкое место, и нужна оптимизация именно тут. Я же не зря в самом начале выразил удивление использованием линкедлиста в этом коде(но мало ли, может правда надо, из этого куска нифига неясно, и контракт этого метода даже близко неизвестен).


А я специально проигнорировал этот момент, потому, что это не существенно и могут быть какие-то причины делать так.

E__>Топикстартер врядли именно это имел ввиду, постя этот код. Полагаю, что что-то не понравилось ему как сишнику, но он молчит как партизан.


Хм, я вроде видел его в жавовском форуме, не считал его новичком. А сишная интуиция в жаве иногда сбоит. Я вот проверил заодно скорость итерации — LinkedList оказался чуть-чуть быстрее (что неудивительно, если подумать, чем отличаются итераторы Java и C++). Мою веру в тотальное превосходство ArrayList сохраняет только меньший даже в худшем случае расход памяти. В жаве иногда свободно используются такие приёмы, которые грамотному сишнику даже в голову не пришли бы (вроде рефлексии или работы с кучей в цикле). Но в жаве это тоже оказыв несущественным.
Re[5]: [Java] best practices
От: vsb Казахстан  
Дата: 13.05.11 06:47
Оценка:
Здравствуйте, Eugeny__, Вы писали:

E__>Здравствуйте, vsb, Вы писали:



UDI>>>LinkedList, видимо, для того чтобы как можно быстрее создать копию m_bandList, так как в это время модификация m_bandList (я предполагаю, что это synchronized обертка над коллекцией) блокируется. И поэтому немаловажно, что все это в synchronized блоке, без него получилось бы фиг знает что. Думаю, назначение метода — потокобезопасное создание копии коллекции m_bandList на текущий момент времени.


vsb>>ArrayList создать быстрее.


E__>Не, создается таки линкованный быстрее. Напиши тестик для большого количества данных — там в разы разница. Но это, опять же, ни о чем не говорит. Мы даже примерно не знаем даже какого размера коллекции предполагаются, и как они будут использоваться. Тот же линкед создать-то быстрее, но если к нему планируется пару обращений, то не беда. Если по нему будет много рандомных обращений по индексу... Ну вы поняли.


        List<Integer> source = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 3, 2, 5, 6, 1, 6);
        
        long start;

        start = System.currentTimeMillis();
        for (int i = 0; i < 10000000; ++i) {
            new LinkedList<Integer>(source);
        }
        System.out.println(System.currentTimeMillis() - start);

        start = System.currentTimeMillis();
        for (int i = 0; i < 10000000; ++i) {
            new ArrayList<Integer>(source);
        }
        System.out.println(System.currentTimeMillis() - start);


Выдаёт
1724
393

Т.е. ArrayList создаётся более чем в 4 раза быстрее. Если увеличивать количество элементов в исходном массиве, то скорость будет различаться ещё больше, но даже для одного элемента ArrayList быстрее.
Re[6]: [Java] best practices
От: Eugeny__ Украина  
Дата: 13.05.11 09:21
Оценка:
Здравствуйте, vsb, Вы писали:


E__>>Не, создается таки линкованный быстрее. Напиши тестик для большого количества данных — там в разы разница. Но это, опять же, ни о чем не говорит. Мы даже примерно не знаем даже какого размера коллекции предполагаются, и как они будут использоваться. Тот же линкед создать-то быстрее, но если к нему планируется пару обращений, то не беда. Если по нему будет много рандомных обращений по индексу... Ну вы поняли.


vsb>
vsb>        List<Integer> source = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 3, 2, 5, 6, 1, 6);
        
vsb>        long start;

vsb>        start = System.currentTimeMillis();
vsb>        for (int i = 0; i < 10000000; ++i) {
vsb>            new LinkedList<Integer>(source);
vsb>        }
vsb>        System.out.println(System.currentTimeMillis() - start);

vsb>        start = System.currentTimeMillis();
vsb>        for (int i = 0; i < 10000000; ++i) {
vsb>            new ArrayList<Integer>(source);
vsb>        }
vsb>        System.out.println(System.currentTimeMillis() - start);
vsb>


vsb>Выдаёт

vsb>1724
vsb>393

vsb>Т.е. ArrayList создаётся более чем в 4 раза быстрее. Если увеличивать количество элементов в исходном массиве, то скорость будет различаться ещё больше, но даже для одного элемента ArrayList быстрее.


Кстати, признаю свою ошибку. В моем тесте была неточность, которую я сразу не заметил. Действительно ArrayList быстрее.
Новости очень смешные. Зря вы не смотрите. Как будто за наркоманами подсматриваешь. Только тетка с погодой в завязке.
There is no such thing as a winnable war.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.