Pull to refresh

Comments 38

Так принято поступать абсолютно во всех языках программирования, даже в C#

В C# есть кортежи:


(long sum, double avg) computeSumAndAverage(..) {...}
Предложение было про определение нескольких классов в одном файле.
Кортежи это хорошо, иногда, но в Java их нет.

Это отчего-то их нет? Pair или Tuple, от которого вы почему-то воротите нос, в общем-то и является кортежем. Вполне статически типизированным, и удобным в работе. А вот тот именованный класс, который вы вместо предлагаете, лично у меня как раз вызывает отторжение — потому что он как раз не может быть как правило повторно использован.

Pair/Tuple не дают никакой информации о том, что же там за данные. Вот функция doOperation() возвращает Pair<String, Integer> — что это за данные? Без документации не разобраться.
Если вернуть класс с полями типа String errorMessage, Integer errorCode — сразу понятно, что же это за зверь.

А они и не должны. Их задача — сообщить тип компонентов пары. Если вам нужно чтобы сообщение об ошибке было не просто String — вот для него и заведите осмысленный класс (которых у вас будет N, а не N*M, как в случае для бессмысленных классов на каждую пару типов). Или даже не заводите — а верните скажем Pair<Integer, Exception>, где будет ваше сообщение об ошибке, стек и еще что-то полезное.

И я попрошу заметить, что изначально просили кортежи. Если вы откроете определение кортежа, то увидите, что там нет никаких имен. Поэтому то что вы просите — это не кортеж.

Как это так, C# — клон Java, а кортежей в ней нет? ;)

try(final ZipInputStream zipInputStream = 
                 new ZipInputStream(
                          new FileInputStream(file), charset)) {...}


В коде выше есть неочевидная ошибка, из за которой FileInputStream может быть не закрыт.

Пожалуйста, поясните свою мысль. Я вижу, что тут стрим zipInputStream закроется обязательно. А он в свою очередь (по правилам упаковки стримов друг в друга) должен закрыть нижележащий FileInputStream.
Несмотря на документацию, которая говорит о необходимости ручного закрытия потоков связанных с IO, FileInputStream реализует интерфейс AutoCloseable. Так что я тоже присоединяюсь к просьбе пояснить мысль.
Мысль в том что Java разработчики не умею решать задачи управления ресурсами,
дело не в том что задача сложная, но в том что в Java с такой проблемой приходится
сталкиваться слишком редко. Необходимый навык не вырабатывается.

Уверен, пытливый читатель может найти ошибку в приведенном мною примере
за пять, максимум десять минут. А когда суть ошибки становится ясна,
возникают логичные вопросы. Почему вопиющая ошибка не была видна сразу?
Что сделать для того чтобы подобные ошибки не повторялись в дальнейшем?

Остальное есть в статье, не буду повторяться.
Мысль в том что Java разработчики не умею решать задачи управления ресурсами

Это гениально :) Поэтому вы не найдете ни одной Java программы, которая может работать непрерывно больше недели — ресурсы утекают из-за неумения управлять ими. dm_wrike, вам ещё многое предстоит узнать для себя на пути изучения Java.

Единственное, что я тут вижу ошибка может произойти в конструкторе ZipInputStream. В частности если charset строка, то может выскочить CharsetUnknown или как-то похоже называющееся исключение… и FileInputStream в такой ситуации не закроется…
Меня вот только смущает, что следующий их пример проблему-то и не решает, ибо там банально один стрим.


PS а с inner-классами вообще беда-беда… они отвратительно влияют на читабельность родительского класса и допустимы в основном тогда, когда являются банальным представлением данных без средств обработки этих данных. Ну т.е. конструктор и геттеры… ну и опционально — сеттеры.

Спасибо. То есть предполагается, что ZipInputStream считает заголовок или вроде того? Но конструктор не бросает таких исключений и в качестве charset ожидает объект Charset, а не строку — CharserUncknown тоже не выбросит. Вот если мы туда null передадим — то вылетит NullPointerException — тогда уже файл действительно не закроется. То есть направление вы абсолютно верное указали.
PS твратительно влияют на читабельность родительского класса — на самом деле это дело привычки.
PS а с inner-классами вообще беда-беда… они отвратительно влияют на читабельность родительского класса и допустимы в основном тогда, когда являются банальным представлением данных без средств обработки этих данных. Ну т.е. конструктор и геттеры… ну и опционально — сеттеры.

Почитайте что такое method-object.
У вас 10 параметров у функции, 9 из них всегда константы, а функцию вы вызываете много раз в цикле.
Вынести в вложенный класс и у вас будет большой конструктор и вызов метода с одним параметром. Это экономит стек.

А пример кода под вашу ситуацию существует? У меня что-то не получается представить себе такую ситуацию без существенного code smell...


И да, для невнимательных, обращаю внимание, что я совсем противник вложенных классов, обладающих действием. Возможность создания "внутреннего представления" я как бы и не отметаю совсем, хотя в большинстве случаев что-то там не так.

Как пример: https://github.com/lastrix/asn1s/blob/master/asn1s-core/src/main/java/org/asn1s/core/type/x680/collection/SequenceType.java


Валидатору требуется для проверки NamedValue 4 обязательных константных аргумента и доступ к данным, класса SequenceType.
Предлагаете, что лучше устанавливать внешний валидатор, когда процедура проверки одна и не меняется? И в принципе не может изменится?
С method-object получается более компактный код.


Есть другой вариант парсер конфигураций 1С. В зависимости от типа объекта в xml, необходимо выполнять свою логику, причем она отличается сильно, но общего очень много.
у базового класса тонна информации, вроде около 10 полей (как констант, так и изменяемых), все необходимы при анализе. Часть создается в процессе анализа.
Раньше весь код представлял из себя кашу разных методов с 5-6 аргументами как минимум, сейчас 3 аргумента максимум и весь код разбит на функциональные блоки — вот конфигурируется документ, вот загружается контент из xml, тут определяются типы.
Про быстродействие даже говорить не хочется, оно просто в разы выше.
Хотя важнее именно повышение читаемости кода.


Или лучше иметь один класс (который в принципе нельзя разбить на независимые друг от друга классы без создания нереального количества мусорного кода), и 100+ методов в нем?


Если не страшно, вот пример из реального, работающего проекта, в исходном виде 1500 строк:


Часть класса
final class MetaDataMapper
{
    /**
        Огромное количество констант
    **/

    MetaDataMapper( VirtualFile file, 1СDocument sdk, 1СDocument target, @Nullable Element root )
    {
        this.file = file;
        this.sdk = sdk;
        this.target = target;
        this.root = root;
        resolver = new OneCv8ClassResolver( target );
        sourceFile = new SourceFileStub( file.getAbsoluteName().substring( 1 ), null );
        dummyDeclareStatement = DeclareStatementModifier.dummy( file );
    }

    private final VirtualFile file;
    private final SourceFileStub sourceFile;
    private final 1СDocument sdk;
    private final Element root;
    private final 1СDocument target;
    private final OneCv8ClassResolver resolver;
    private final DeclareStatementModifier dummyDeclareStatement;
    private ClassBuilder classBuilder;
    private List<ClassBuilder> transformationClasses;

    void mapElement()
    {
        if( file.getName().startsWith( "Subsystem." )
                || file.getName().startsWith( "WSReference." )
                || file.getName().startsWith( "CommonAttribute." ) )
            return;

        if( root.getTagName().equalsIgnoreCase( "Configuration" ) )
        {
            target.newClass( "System.Конфигурация.ЭтаКонфигурация" )
                    .parent( TYPE_CONFIGURATION )
                    .superClass( TYPE_CONFIGURATION.classReference() )
                    .declareStatement( asStatement( root ) )
                    .customData( ATTR_UUID, root.getAttribute( ATTR_UUID ) )
                    .build();
            return;
        }

        Element properties = XmlUtils.getSingleElementOrDie( root, TAG_PROPERTIES );
        Element innerInfo = XmlUtils.getSingleElementOrNull( root, TAG_INNER_INFO );
        Element internalInfo = XmlUtils.getSingleElementOrNull( root, TAG_INTERNAL_INFO );
        Element childObjects = XmlUtils.getSingleElementOrNull( root, TAG_CHILD_OBJECTS );
        Element standardAttributes = XmlUtils.getSingleElementOrNull( properties, TAG_STANDARD_ATTRIBUTES );

        resolveThisClass();
        transformationClasses = new ArrayList<>();
        if( innerInfo != null || internalInfo != null )
            new ClassTransformer( innerInfo, internalInfo ).doTransform();

        if( childObjects != null )
            readChildren( childObjects, standardAttributes, classBuilder, transformationClasses );

        if( properties != null && root.getTagName().equalsIgnoreCase( "Document" ) )
            registerDocumentSpecials( properties );

        notifyComplete();
    }

    void mapFormElement()
    {
        if( !root.getTagName().equalsIgnoreCase( "Form" ) )
            throw new IllegalStateException();

        resolveThisClass();
        FormAttributeReader attributeReader = new FormAttributeReader( classBuilder, transformationClasses );
        readFormAttrs( attributeReader, TAG_ATTRIBUTES, TAG_ATTRIBUTE );
        readFormAttrs( attributeReader, TAG_PARAMETERS, TAG_PARAMETER );

        Element elements = XmlUtils.getSingleElementOrNull( root, TAG_ELEMENTS );
        if( elements != null )
            new FormElementsReader( elements ).read();

        Element commands = XmlUtils.getSingleElementOrNull( root, "Commands" );
        if( commands != null )
            new FormCommandsReader( XmlUtils.getChildren( commands, "Command" ), asStatement( commands ) ).read();
        notifyComplete();
    }

    void mapPredefined()
    {
        if( !root.getTagName().equalsIgnoreCase( "predefinedData" ) )
            throw new IllegalStateException();

        resolveThisClass();

        for( Element item : XmlUtils.getChildren( root, "item" ) )
        {
            String name = XmlUtils.getTextContentOrDie( item, "name" );
            Element codeElement = XmlUtils.getSingleElementOrNull( item, "code" );
            String attrType = null;
            if( codeElement != null )
                attrType = codeElement.getAttribute( "xsi:type" );

            1CType type = StringUtils.isBlank( attrType )
                    ? getFieldType( item, "type" )
                    : resolver.resolveType( Collections.singletonList( attrType ) );

            classBuilder.field( name ).type( type ).declareStatement( asStatement( item ) ).build();
        }
        notifyComplete();
    }

    void mapTxtFile()
    {
        1CClassReference reference = resolver.buildClassStructure( file.getName() );
        if( reference == null )
            throw new IllegalStateException( "Unable to resolve class: " + file );

        classBuilder = target
                .editClass( reference )
                .modifier( dummyDeclareStatement );

        transformationClasses = new ArrayList<>();
        new PlainClassTransformer().doTransform();
        notifyComplete();
    }

    private void resolveThisClass()
    {
        1CClassReference reference = resolver.buildClassStructure( file.getName() );
        if( reference == null )
            throw new IllegalStateException( "Unable to resolve class: " + file.getName() );

        classBuilder = target.editClass( reference );

        Element properties = XmlUtils.getSingleElementOrNull( root, TAG_PROPERTIES );
        if( !root.getTagName().equals( "predefinedData" ) )
            classBuilder.declareStatement( asStatement( root ) )
                    .customData( ATTR_UUID, root.getAttribute( ATTR_UUID ) );

        String tagName = root.getTagName();
        if( FORMS.contains( tagName.toLowerCase() ) && isManagedForm() )
        {
            classBuilder.parent( TYPE_MANAGED_FORM )
                    .modifier( Modifiers.ENFORCE_PARENT_CLASS );
        }
        else if( properties != null && tagName.equalsIgnoreCase( "Constant" ) )
            registerConstantValueField( properties );
        else if( properties != null && tagName.equalsIgnoreCase( "SessionParameter" ) )
            registerSessionParameter( properties );
    }

    private void notifyComplete()
    {
        classBuilder.build();
        if( transformationClasses != null )
            transformationClasses.forEach( ClassBuilder:: build );
    }

    private void registerDocumentSpecials( Node properties )
    {
        ClassBuilder newRegisterRecordsCollection = createRecordsCollection();
        createSpecialFields( newRegisterRecordsCollection );

        Element registerRecords = XmlUtils.getSingleElementOrNull( properties, "RegisterRecords" );
        if( registerRecords != null )
            for( Element element : XmlUtils.getChildren( registerRecords, "xr:item" ) )
                registerDocumentSpecialMember( newRegisterRecordsCollection, element );

        newRegisterRecordsCollection.build();
    }

    private void registerDocumentSpecialMember( ClassBuilder newRegisterRecordsCollection, Node element )
    {
        String typeName = element.getTextContent();
        1CClassReference reference = resolver.buildClassStructure( typeName );
        1CClass aClass = reference == null ? null : target.getClassOrNull( reference );
        if( aClass == null )
            log.warn( "Failed to resolve: " + typeName );
        else
        {
            String name = aClass.getName();
            String memberTypeName =
                    OneCv8Types.NAMESPACE + '.'
                            + aClass.getParentClass().classReference().getName() + "НаборЗаписей." + name;

            1CType memberType = OneCv8Types.parse( memberTypeName );
            newRegisterRecordsCollection
                    .field( name )
                    .type( memberType )
                    .declareStatement( asStatement( element ) )
                    .buildField();
        }
    }

    private void createSpecialFields( ClassBuilder newRegisterRecordsCollection )
    {
        1CType type = newRegisterRecordsCollection.getReference().to1CType();
        for( ClassBuilder aClass : transformationClasses )
            if( REF_DOCUMENT_OBJECT.equals( aClass.getSuperClass() ) )
                aClass
                        .field( "Движения" )
                        .type( type )
                        .declareStatement( aClass.getDeclareStatement() )
                        .build();
    }

    @NotNull
    private ClassBuilder createRecordsCollection()
    {
        1CClassReference newRegisterRecordsCollectionReference =
                OneCv8Language.reference( REF_RECORDS_COLLECTION.getFullName() + '.' + classBuilder.getReference().getName() );

        if( target.getClassOrNull( newRegisterRecordsCollectionReference ) == null )
            return target.newClass( newRegisterRecordsCollectionReference )
                    .superClass( REF_RECORDS_COLLECTION )
                    .parent( REF_RECORDS_COLLECTION.to1CType() );

        return target.editClass( newRegisterRecordsCollectionReference );
    }

    private void registerSessionParameter( Node properties )
    {
        ClassBuilder paramClassBuilder = target.editClass( REF_SESSION_PARAMETERS );
        String name = Utils.getObjectName( properties );
        1CType type = getFieldType( properties, TAG_TYPE );
        paramClassBuilder
                .field( name ).type( type ).declareStatement( asStatement( root ) ).build()
                .build();
    }

    private void registerConstantValueField( Node properties )
    {
        1CType fieldType = getFieldType( properties, TAG_TYPE );
        classBuilder.field( "Значение" )
                .type( fieldType )
                .declareStatement( asStatement( root ) )
                .build();
    }

    private boolean isManagedForm()
    {
        Element properties = XmlUtils.getSingleElementOrNull( root, TAG_PROPERTIES );
        String formType = properties == null ? null : XmlUtils.getTextContentOrNull( properties, "FormType" );
        return "managed".equalsIgnoreCase( formType );
    }

    private void readFormAttrs( FormAttributeReader attributeReader, String tagName, String childTagName )
    {
        Element attributes = XmlUtils.getSingleElementOrNull( root, tagName );
        if( attributes != null )
            for( Element element : XmlUtils.getChildren( attributes, childTagName ) )
                attributeReader.read( element );
    }

    private void readChildren( Node childObjects, @Nullable Node standardAttributes, ClassBuilder primaryClass, List<ClassBuilder> secondaryClasses )
    {
        AbstractChildReader classFieldReader = new ClassFieldReader( primaryClass, secondaryClasses );
        AbstractChildReader tabularSectionReader = new TabularSectionReader( primaryClass, secondaryClasses );
        AbstractChildReader commandReader = new CommandReader( primaryClass, secondaryClasses );

        Collection<Element> children = XmlUtils.getChildren( childObjects );
        for( Element child : children )
        {
            String tagName = child.getTagName();
            if( ALLOWED_CLASS_FIELDS.contains( tagName.toLowerCase() ) )
                classFieldReader.read( child );
            else if( TAG_TABULAR_SECTION.equalsIgnoreCase( tagName ) )
                tabularSectionReader.read( child );
            else if( TAG_COMMAND.equalsIgnoreCase( tagName ) )
                commandReader.read( child );
            else if( !SKIPPED_CLASS_FIELDS.contains( tagName.toLowerCase() ) )
                log.error( "Skipping child object of unknown type - " + tagName );
        }

        if( standardAttributes != null )
        {
            Collection<Element> attributes = XmlUtils.getChildren( standardAttributes );
            StandardAttributeReader reader = new StandardAttributeReader( primaryClass, secondaryClasses );
            attributes.forEach( reader:: readAttribute );
        }
    }

    private 1CType getFieldType( Node properties, String typeTagName )

    private IStatement asStatement( Node node )

    private final class FormElementsReader
    {
        private final Element elements;
        private final ClassBuilder elementsSubClass;
        private final 1CClassReference parentClassReference;
        private final Deque<1CClassReference> eventTypeSource = new LinkedList<>();

        void read()

        void read( Node elementsNode )
    }

    private final class FormCommandsReader
    {
        private final List<Element> commands;
        private final ClassBuilder formCommands;

        void read()

    }

    private final class FormAttributeReader extends AbstractChildReader
    {
        @Override
        void read( Element element )
    }

    private final class SubTableAttributeReader extends AbstractChildReader
    {
        private final String className;
        private final 1CType parentClass;
        private final 1CClassReference pathToColumns;

        @Override
        void read( Element element )
    }

    private final class StandardAttributeReader
    {
        private static final String CUSTOM_DATA_KEY_ALIAS = "alias";

        private final ClassBuilder primaryClass;
        private final List<ClassBuilder> secondaryClasses;

        void readAttribute( Element attribute )
    }

    private static final class AttributeTemplate
    {
        private String name;
        private 1CType type;
        private final Collection<Modifier> modifiers = new HashSet<>();

        void addModifier( Modifier modifier )

        void setType( 1CType type )

        public void setName( String name )

        String getName()

        1CType getType()

        Collection<Modifier> getModifiers()
    }

    private abstract class AbstractChildReader
    {
        private final ClassBuilder primaryClass;
        private final Collection<ClassBuilder> secondaryClasses;

        ClassBuilder getPrimaryClass()

        Collection<ClassBuilder> getSecondaryClasses()

        1CType getXmlFieldType( Node properties, boolean useClassType, String typeTagName )

        abstract void read( Element element );

        void forEachClass( Consumer<ClassBuilder> callback )
    }

    private final class ClassFieldReader extends AbstractChildReader
    {
        @Override
        void read( Element element )
    }

    private final class TabularSectionReader extends AbstractChildReader
    {
        @Override
        void read( Element element )
    }

    private final class CommandReader extends AbstractChildReader
    {
        @Override
        void read( Element element )
    }

    private final class ParentMembersReplacer
    {
        private final ClassBuilder currentClass;
        private final Collection<1CClassReference> typesToReplace;
        private final 1CClass parent;
        private boolean typeReplaced;

        void copyMembersFromParent()

    }

    private class PlainClassTransformer
    {
        private ClassBuilder current;

        void doTransform()

        void doPrimaryClassTransformations()

        void produceType( String name, @Nullable String uuid, Modifier declareStatementModifier )

        void registerField( 1CClassReference classReference )

        void registerSubClass( String subClassParentName )
    }

    private final class ClassTransformer extends PlainClassTransformer
    {
        private final Element innerInfo;
        private final Element internalInfo;

        @Override
        void doTransform()
    }
}

Внутренности вложенных классов вырезал специально.


Работает это быстрее, чем прошлая реализация примерно в 2-3 раза (из-за других оптимизаций получилось намного выше). Просто потому что стэк не нагружается избыточной и ненужной работой, не говоря о том, что методы стали не по 100-150 строк в среднем, а по 50 максимум.
И загрузка десятка тысяч файлов xml не за 5 минут осуществляется, а за <30 секунд.


А вы говорите, что вложенные классы никогда нельзя применять.
С вашим подходом и синглтон — это убогий костыль, за который надо сажать и отбирать диплом.


Может все дело в том, что применять надо уметь инструмент? Знать где оно применимо?

вот буквально вчера видел(правда не ява)
преобразование координат на карте
параметры — собсно координата и 6 параметров системы координат, много раз подряд они ессн оодинаковые
UFO just landed and posted this here
Как сделать Java код проще и нагляднее

Переписать на Kotlin?


Простите, не удержался :)

Но чем заменять Kotlin, он же становится все больше и больше Scala…
Аналогично в языке Go, все определения по умолчанию являются константами:
greeting := “Hello”

Для определения переменной нужно опять же указать дополнительное слово:
var answer := “Hi”

tour.golang.org/basics/10

Краткая форма объявления переменных
Внутри функции, краткий оператор присваивания := с неявным типом может быть использован вместо объявления с помощью var
Да, вы правы, косяк. А я то был лучшего мнения о Go.
isSupportedBrowser = ((Supplier<boolean>)()->{ … }).get();

этим вы предлагаете заменить хорошо названный метод?
Хороший метод зачем же заменять. А вот если у вас N-цать строк посреди метода вычисляют одно значение, можно и в Supplier завернуть.
Так почему же из него метод не сделать? если это N строк, которые вычисляют определенное значение и например могут быть протестированы и вообще могут не мозолить глаза своими деталями?
Возможно вас путает пример с определением типа браузера, поскольку кажется что эта логика может быть переиспользована, подразумевается что это не так и код одноразовый. Возможно не слишком удачный пример.

Тем не менее, вы апеллируете к тому что нужно вынести метод, но пример про другое.

У вас есть переменная, для ее вычисления вам нужно определить несколько дополнительных переменных которые далее вам не нужны.

Можно просто написать вычисления в коде метода, тогда читая дальнейшую логику
вам придется иметь в виду что есть еще дополнительные переменные, а потом окажется
что для дальнейшей логики они не нужны.

Либо, можно спрятать такие вычисления в Supplier, и тем самым показать что
про все промежуточные переменные дальше можно забыть, не только компилятору,
но и тому кто читает код.
кажется что эта логика может быть переиспользована

вы так говорите, как будто логику в методы выносят только для переиспользования
Либо, можно спрятать такие вычисления в Supplier

или намного проще, визуально понятнее и быстрее «спрятать» их в метод. Понимаю, вы подсмотрели этот паттрен в джаваскрипте, но в джаве он слишком громоздкий. В котлине еще как-то можно было бы что-то такое с инлайн-функциями провернуть
val result = run { "got it" }
и то редко такое может пригодиться.
Получается, вы имеете в виду примерно следующее:
   ...
   final Тип имяЗначения = вычислитьИмяЗначения(параметр1, параметр2, ...);


Это может быть вполне хорошим способом структурировать код,
однако:
1. Имя метода типично будет дублировать имя значения и не несет нового смысла.
2. Для того чтобы понять как вычисляется значение нужно будет перейти
в тело метода, посмотреть логику там, потом вернуться и продолжить читать
основной метод. Муторно для одноразового метода.
3. Если параметров для вычисления много, одноразовый метод становится
еще и громоздким. Нужно будет не только придумать имена для всех
параметров но и держать их в голове при чтении.

Либо:
final Тип имяЗначения = ((Supplier<Тип>)()->{ … }).get();
Вот именно за такие советы как сделать код «проще» и считают Java плохим языком.

1)
new Runnable() {...}.run(); 


Анонимный класс, да с вызовом сразу после этого метода — тихий ужас. Читая такой код можно голову сломать к чем относится тот или иной метод, как происходит выполнение и т.д. Не говоря уже о избыточности и многословности.
2)
final boolean isSupportedBrowser = ((Supplier<Boolean>)()->{...}).get();


Бррр. Простая функция в разы проще и понятнее этих безумных скобок, приведений и get'ов. Да, может быть приватная функция, которая используется только один раз, только чтобы декомпозировать код (см. «чистый код»), это намного лучше чем подобное. Явный пример попытки забивать гвозди микроскопом.

Читать такой код сложно, не сразу понятно, что там идет получения значение, начинающий разработчик сломает голов пытаясь понять такой код.

3)
{ //transform report header:
		final boolean someFlagUsedOnlyHere = … 
		… 50 lines of code … 
	}


Если у вас возникает такая потребность значит у вас что-то с кодом неправильно, стоит выделить функции или внутренние классы и работать с ними. Функция с правильным названием намного лучше такого блока с комментарием. Практически всегда это можно сделать.

4)
При этом аккуратное решение совсем простое:

public class MathExt {
	public static class SumAndAverage {
		private final long sum;
		private final double average;
	
		public SumAndAverage(final long sum, final double average) {...}

		public long getSum() {...}
		public double getAverage() {...}
	}

	public static SumAndAverage computeSumAndAverage(final ImmutableList<Integer> valueList) {
		...
	} 
}


Иногда это имеет смысл, но в большинстве случаев, если SumAndAverage будет часто использоваться в других классах, у вас будут либо статические импорты (что не очень рекомендуется), либо некрасивый и длинный код MathExt.SumAndAverage во многих местах. В большинстве случаев лучше либо использовать Pair или его аналог, если это временные объекты, которые сразу будут удалены после получения, либо делать SumAndAverage отдельным DTO классом.

Если значения использовать и присваивать сразу, то разницы практически нет:
MathExt.SumAndAverage sumAndAverage =…
int sum = sumAndAverage.getSum();
double average = sumAndAverage.getAverage();



И
Pair<Integer,Double> sumAndAverage =…
int sum = sumAndAverage.getFirst();
double average = sumAndAverage.getSecond();


Мы и так видим, что это сумма и среднее. А на генерацию и сопровождение такого количества внутренних классов тратится время и ухудшается читабельность самого класса.

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


Обратите внимание, я нигде не утверждаю что приведенные мною техники написания кода
являются единственно правильными. Наоборот, я подчеркиваю что они скорее отличаются
от общепринятого подхода к написанию Java кода.

Каждый разработчик при написании кода использует определенные приемы,
и код написанный подобным образом ему, само собой, кажется простым и понятным.

Приведенные мною примеры не являются типичными, так что на первый взгляд может
показаться что они хуже читаются.

Приведенные выше примеры позволяют сделать код чуть менее сложным и чуть более выразительным. По крайней мере, для нас они полезны.


Надеюсь что они окажутся полезны и для кого то еще.
Но для этого нужно изменить некоторые привычки написания и чтения кода.
Это сложно, и вообще говоря, не обязательно.

Pair нарушает инкапсуляцию. Это знание, какое поле что означает, все равно есть, только в неявном виде. Гораздо более правильно (true OOP ;) ) это знание определить явно, в виде класса. Скорее всего, оно все скаляризуется и класса там не будет вообще. Для уменьшения кода мы используем Lombok.

Pair нарушает инкапсуляцию чего? Класса, который его создал? Это не так, он вообще не обязан как-то быть связан с этим классом. Своих полей внутри — он для этого и создан, можно еще Map.Entity вспомнить или всякие функции из Stream Api.

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

По-хорошему внутренний класс не очень-то предназначен для того чтобы его использовать в других местах, потому что он создает сильную связанность между классом, который его сгенерил и всеми классами, которые его используют.

Пример — вы создали внутренний класс MathExt.SumAndAverage и так получилось, что он оказался использованным во всех классах приложения, потом вы решили добавить совсем новый класс MathExt2, который возвращает тот же SumAndAverage, но теперь без рефакторинга ВСЕХ классов приложения вы не сможете просто использовать MathExt2, потому что SumAndAverage привязан жестко к MathExt. Это сложно назвать хорошим дизайном. Поэтому оптимально либо Pair — аргумент «простой код лучше, чем сложный», либо отдельный класс DTO, не связанный напрямую с классом его создающим.

Да, нарушает инкапсуляцию создающего класса: чтобы понять, что означают поля Pair, нужно будет заглянуть внутрь создающего класса. Либо же тогда делать Pair частью контракта и описывать явно в JavaDoc. Но если это часть контракта, тогда надо делать это явно и завести ещё один DTO.

Касательно этого new Runnable(){...}.run() — просто автор не смог сделать обход графа без рекурсивного вызова, а про возможность декларирования своего класса внутри Java метода — не знает. Ну ничего — ещё научится разворачивать рекурсии в циклы и наоборот, а классы внутри методов лучше не объявлять ;).

Вообще, в статье много неправильных вещей (с тем же «нескучным скучным кодом», который луше было бы разбить на отдельные методы вообще-то, а не со скобками играться). Но разбор всех ляпов превратится в полноценную статью.
Sign up to leave a comment.