سؤال

هناك شيء غير مرضٍ للغاية بشأن هذا الرمز:

/*
Given a command string in which the first 8 characters are the command name
padded on the right with whitespace, construct the appropriate kind of 
Command object.
*/
public class CommandFactory {
     public Command getCommand(String cmd) {
         cmdName = cmd.subString(0,8).trim();

         if(cmdName.equals("START")) {
             return new StartCommand(cmd);
         }
         if(cmdName.equals("END")) {
             return new EndCommand(cmd);
         }
         // ... more commands in more if blocks here
         // else it's a bad command.
         return new InvalidCommand(cmd);
     }
}

أنا غير نادم على نقاط الخروج المتعددة، فالهيكل واضح.لكنني لست سعيدًا بسلسلة عبارات if شبه المتطابقة.لقد فكرت في إنشاء خريطة للسلاسل للأوامر:

commandMap = new HashMap();
commandMap.put("START",StartCommand.class);
// ... etc.

...ثم استخدم الانعكاس للبحث عن مثيلات الفئة المناسبة من الخريطة.ومع ذلك، على الرغم من كونه أنيقًا من الناحية النظرية، إلا أن هذا يتضمن قدرًا لا بأس به من كود الانعكاس الذي قد لا يقدره أي شخص يرث هذا الكود - على الرغم من أن هذه التكلفة قد يتم تعويضها بالفوائد.جميع قيم الترميز الثابت للأسطر في CommandMap لها رائحة سيئة تقريبًا مثل كتلة if.

سيكون من الأفضل أن يتمكن منشئ المصنع من فحص مسار الفصل بحثًا عن فئات فرعية من الأوامر، والاستعلام عنها لتمثيلات السلسلة، وإضافتها تلقائيًا إلى ذخيرته.

إذن - كيف يجب أن أقوم بإعادة هيكلة هذا؟

أعتقد أن بعض الأطر المتوفرة تعطيني هذا النوع من الأشياء مجانًا.لنفترض أنني لست في وضع يسمح لي بترحيل هذه الأشياء إلى مثل هذا الإطار.

هل كانت مفيدة؟

المحلول

أعتقد أن خريطة السلاسل الخاصة بك للأوامر جيدة.يمكنك أيضًا استبعاد اسم أمر السلسلة إلى المُنشئ (أي:ألا ينبغي أن يعرف StartCommand أن الأمر الخاص به هو "START"؟) إذا كان بإمكانك القيام بذلك، فسيكون إنشاء كائنات الأوامر الخاصة بك أسهل بكثير:

Class c = commandMap.get(cmdName);
if (c != null)
    return c.newInstance();
else
    throw new IllegalArgumentException(cmdName + " is not as valid command");

خيار آخر هو إنشاء enum لجميع أوامرك التي تحتوي على روابط للفئات (افترض أن جميع كائنات الأوامر الخاصة بك تنفذ CommandInterface):

public enum Command
{
    START(StartCommand.class),
    END(EndCommand.class);

    private Class<? extends CommandInterface> mappedClass;
    private Command(Class<? extends CommandInterface> c) { mappedClass = c; }
    public CommandInterface getInstance()
    {
        return mappedClass.newInstance();
    }
}

نظرًا لأن toString الخاص بالتعداد هو اسمه، فيمكنك استخدامه EnumSet لتحديد موقع الكائن الصحيح والحصول على الفصل من الداخل.

نصائح أخرى

ماذا عن الكود التالي:

public enum CommandFactory {
    START {
        @Override
        Command create(String cmd) {
            return new StartCommand(cmd);
        }
    },
    END {
        @Override
        Command create(String cmd) {
            return new EndCommand(cmd);
        }
    };

    abstract Command create(String cmd);

    public static Command getCommand(String cmd) {
        String cmdName = cmd.substring(0, 8).trim();

        CommandFactory factory;
        try {
            factory = valueOf(cmdName);
        }
        catch (IllegalArgumentException e) {
            return new InvalidCommand(cmd);
        }
        return factory.create(cmd);
    }
}

ال valueOf(String) يتم استخدام التعداد للعثور على طريقة المصنع الصحيحة.إذا كان المصنع غير موجود فسوف يرمي IllegalArgumentException.يمكننا استخدام هذا كإشارة لإنشاء InvalidCommand هدف.

فائدة إضافية هي أنه إذا كنت تستطيع أن تفعل هذه الطريقة create(String cmd) public إذا كنت ستجعل أيضًا هذه الطريقة لإنشاء وقت ترجمة كائن الأمر المحدد متاحة لبقية التعليمات البرمجية الخاصة بك.يمكنك بعد ذلك استخدام CommandFactory.START.create(String cmd) لإنشاء كائن أمر.

الميزة الأخيرة هي أنه يمكنك بسهولة إنشاء قائمة بجميع الأوامر المتاحة في وثائق Javadoc الخاصة بك.

باستثناء

cmd.subString(0,8).trim();

جزء، وهذا لا يبدو سيئا للغاية بالنسبة لي.يمكنك استخدام الخريطة واستخدام الانعكاس، ولكن اعتمادًا على عدد مرات إضافة/تغيير الأوامر، قد لا يفيدك هذا كثيرًا.

ربما ينبغي عليك توثيق سبب رغبتك في الأحرف الثمانية الأولى فقط، أو ربما تغيير البروتوكول بحيث يكون من الأسهل معرفة أي جزء من هذه السلسلة هو الأمر (على سبيل المثال،ضع علامة مثل ":" أو "؛" بعد كلمة مفتاح الأمر).

إنها ليست إجابة مباشرة على سؤالك، ولكن لماذا لا تقوم بطرح InvalidCommandException (أو شيء مشابه)، بدلاً من إرجاع كائن من النوع InvalidCommand؟

ما لم يكن هناك سبب لعدم إمكانية ذلك، أحاول دائمًا جعل تطبيقات الأوامر الخاصة بي عديمة الحالة.إذا كان الأمر كذلك، فيمكنك إضافة طريقة معرف منطقي (معرف السلسلة) إلى واجهة الأوامر الخاصة بك والتي ستحدد ما إذا كان يمكن استخدام هذا المثيل لمعرف السلسلة المحدد.إذن قد يبدو مصنعك بهذا الشكل (ملاحظة:لم أقم بتجميع أو اختبار هذا):

public class CommandFactory {
    private static List<Command> commands = new ArrayList<Command>();       

    public static void registerCommand(Command cmd) {
        commands.add(cmd);
    }

    public Command getCommand(String cmd) {
        for(Command instance : commands) {
            if(instance.identifier(cmd)) {
                return cmd;
            }
        }
        throw new CommandNotRegisteredException(cmd);
    }
}

تعجبني فكرتك، ولكن إذا كنت تريد تجنب الانعكاس، فيمكنك إضافة مثيلات إلى HashMap بدلاً من ذلك:

commandMap = new HashMap();
commandMap.put("START",new StartCommand());

عندما تحتاج إلى أمر، ما عليك سوى استنساخه:

command = ((Command) commandMap.get(cmdName)).clone();

وبعد ذلك، قمت بتعيين سلسلة الأمر:

command.setCommandString(cmdName);

لكن استخدام clone() لا يبدو أنيقًا مثل استخدام الانعكاس :(

سيكون اتباع نهج Convetion vs Configuration واستخدام الانعكاس للبحث عن كائنات الأوامر المتاحة وتحميلها في خريطتك هو الحل الأمثل.لديك بعد ذلك القدرة على كشف الأوامر الجديدة دون إعادة ترجمة المصنع.

هناك طريقة أخرى للعثور على الفئة المراد تحميلها ديناميكيًا وهي حذف الخريطة الصريحة ومحاولة إنشاء اسم الفئة من سلسلة الأمر.يمكن لحالة العنوان والخوارزمية المتسلسلة تشغيل "START" -> "com.mypackage.commands.StartCommand"، واستخدام الانعكاس فقط لمحاولة إنشاء مثيل له.فشل بطريقة ما (مثيل InvalidCommand أو استثناء خاص بك) إذا لم تتمكن من العثور على الفصل.

ثم تقوم بإضافة الأوامر فقط عن طريق إضافة كائن واحد والبدء في استخدامه.

أحد الخيارات هو أن يكون لكل نوع أمر مصنع خاص به.وهذا يمنحك ميزتين:

1) لن يتصل مصنعك العام بالجديد.لذا، يمكن لكل نوع أمر في المستقبل أن يُرجع كائنًا من فئة مختلفة وفقًا للوسيطات التي تتبع المساحة المتروكة في السلسلة.

2) في مخطط HashMap الخاص بك، يمكنك تجنب الانعكاس عن طريق التعيين إلى كائن ينفذ واجهة SpecialisedCommandFactory، لكل فئة أمر، بدلاً من التعيين إلى الفئة نفسها.من المحتمل أن يكون هذا الكائن عمليًا مفردًا، ولكن لا يلزم تحديده على هذا النحو.يقوم getCommand العام الخاص بك بعد ذلك باستدعاء getCommand المتخصص.

ومع ذلك، فإن انتشار المصنع يمكن أن يخرج عن السيطرة، والرمز الذي لديك هو أبسط شيء يقوم بهذه المهمة.أنا شخصياً ربما أترك الأمر كما هو:يمكنك مقارنة قوائم الأوامر في المصدر والمواصفات دون اعتبارات غير محلية مثل ما كان يُطلق عليه سابقًا CommandFactory.registerCommand، أو الفئات التي تم اكتشافها من خلال التفكير.انها ليست مربكة.من غير المرجح أن يكون بطيئًا لأقل من ألف أمر.المشكلة الوحيدة هي أنه لا يمكنك إضافة أنواع أوامر جديدة دون تعديل المصنع.لكن التعديل الذي ستجريه بسيط ومتكرر، وإذا نسيت إجراؤه، فستحصل على خطأ واضح فيما يتعلق بأسطر الأوامر التي تحتوي على النوع الجديد، لذا فهو ليس مرهقًا.

إن وجود رمز إنشاء الكائن المتكرر هذا مخفيًا في المصنع ليس أمرًا سيئًا للغاية.إذا كان لا بد من القيام بذلك في مكان ما، على الأقل كل شيء هنا، لذلك لن أقلق بشأنه كثيرًا.

اذا أنت حقًا تريد أن تفعل شيئًا حيال ذلك، ربما تختار الخريطة، ولكن قم بتكوينها من ملف الخصائص، وقم ببناء الخريطة من ملف الدعائم هذا.

بدون اتباع مسار اكتشاف مسار الفصل (الذي لا أعرف عنه)، ستقوم دائمًا بتعديل مكانين:كتابة فصل دراسي، ثم إضافة تعيين في مكان ما (المصنع، أو خريطة init، أو ملف الخصائص).

بالتفكير في هذا، يمكنك إنشاء فئات إنشاء مثيل صغيرة، مثل:

class CreateStartCommands implements CommandCreator {
    public bool is_fitting_commandstring(String identifier) {
        return identifier == "START"
    }
    public Startcommand create_instance(cmd) {
        return StartCommand(cmd);
    }
}

بالطبع، يضيف هذا مجموعة كاملة من الفصول الدراسية الصغيرة التي لا يمكنها فعل أكثر من قول "نعم، هذا هو البدء، أعطني ذلك" أو "لا، لا يعجبني ذلك"، ومع ذلك، يمكنك الآن إعادة صياغة المصنع تحتوي على قائمة بهؤلاء CommandCreators واسأل كل منهم:"هل تحب هذا الأمر؟" وإرجاع نتيجة create_instance من أول أمر قبول.بالطبع يبدو الآن أنه من الصعب نوعًا ما استخراج الأحرف الثمانية الأولى خارج CommandCreator، لذا سأعيد صياغة ذلك حتى تتمكن من تمرير سلسلة الأمر بأكملها إلى CommandCreator.

أعتقد أنني قمت بتطبيق بعض "استبدال التبديل بتعدد الأشكال" - إعادة البناء هنا، في حال تساءل أي شخص عن ذلك.

سأختار الخريطة والإنشاء من خلال التفكير.إذا كان فحص مسار الفصل بطيئًا جدًا، فيمكنك دائمًا إضافة تعليق توضيحي مخصص إلى الفصل، وتشغيل معالج التعليقات التوضيحية في وقت الترجمة وتخزين جميع أسماء الفئات في بيانات تعريف الجرة.

ثم الخطأ الوحيد الذي يمكنك القيام به هو نسيان التعليق التوضيحي.

لقد فعلت شيئًا كهذا منذ فترة باستخدام maven و ملائم.

الطريقة التي أفعل بها ذلك هي عدم وجود طريقة مصنع عامة.

أحب استخدام كائنات المجال ككائنات الأوامر الخاصة بي.نظرًا لأنني أستخدم Spring MVC، يعد هذا أسلوبًا رائعًا منذ طريقة DataBinder.setAllowedFields يتيح لي قدرًا كبيرًا من المرونة في استخدام كائن مجال واحد لعدة أشكال مختلفة.

للحصول على كائن أمر، لدي طريقة مصنع ثابتة في فئة كائن المجال.على سبيل المثال، في فئة الأعضاء، سيكون لدي أساليب مثل -

public static Member getCommandObjectForRegistration();
public static Member getCommandObjectForChangePassword();

وما إلى ذلك وهلم جرا.

لست متأكدًا من أن هذا أسلوب رائع، ولم أره مطلقًا مقترحًا في أي مكان وقد توصلت إليه للتو بمفردي، وأنا أحب فكرة الاحتفاظ بأشياء مثل هذه في مكان واحد.إذا رأى أي شخص أي سبب للاعتراض، يرجى إبلاغي بذلك في التعليقات...

أود أن أقترح تجنب التفكير إذا كان ذلك ممكنًا.إنه شرير إلى حد ما.

يمكنك جعل التعليمات البرمجية الخاصة بك أكثر إيجازًا باستخدام العامل الثلاثي:

 return 
     cmdName.equals("START") ? new StartCommand  (cmd) :
     cmdName.equals("END"  ) ? new EndCommand    (cmd) :
                               new InvalidCommand(cmd);

يمكنك تقديم التعداد.إن جعل كل تعداد ثابت في المصنع هو أمر مطول وله أيضًا بعض تكلفة الذاكرة في وقت التشغيل.ولكن يمكنك البحث بسهولة عن التعداد ثم استخدامه مع == أو التبديل.

 import xx.example.Command.*;

 Command command = Command.valueOf(commandStr);
 return 
     command == START ? new StartCommand  (commandLine) :
     command == END   ? new EndCommand    (commandLine) :
                        new InvalidCommand(commandLine);

اذهب مع أمعائك وتأمل.ومع ذلك، في هذا الحل، من المفترض الآن أن يكون لدى واجهة الأوامر الخاصة بك إمكانية الوصول إلى طريقة setCommandString(String s)، بحيث يكون newInstance قابلاً للاستخدام بسهولة.أيضًا، CommandMap هو أي خريطة تحتوي على مفاتيح سلسلة (cmd) لمثيلات فئة الأوامر التي تتوافق معها.

public class CommandFactory {
     public Command getCommand(String cmd) {
        if(cmd == null) {
            return new InvalidCommand(cmd);
        }

        Class commandClass = (Class) commandMap.get(cmd);

        if(commandClass == null) {
            return new InvalidCommand(cmd);
        }

        try {
            Command newCommand = (Command) commandClass.newInstance();
            newCommand.setCommandString(cmd);
            return newCommand;
        }
        catch(Exception e) {
            return new InvalidCommand(cmd);
     }
}

همم، التصفح، وفقط جئت عبر هذا.هل لا يزال بإمكاني التعليق؟

IMHO هناك لا شئ خطأ في رمز كتلة if/else الأصلي.هذا أمر بسيط، ويجب أن تكون البساطة دائمًا هي دعوتنا الأولى في التصميم (http://c2.com/cgi/wiki?DoTheSimplestThingThatCouldPossibleWork)

يبدو هذا صحيحًا بشكل خاص نظرًا لأن جميع الحلول المقدمة أقل توثيقًا ذاتيًا بكثير من الكود الأصلي...أعني ألا ينبغي لنا أن نكتب الكود الخاص بنا للقراءة بدلاً من الترجمة...

على أقل تقدير، يجب أن يحتوي الأمر الخاص بك على getCommandString() -- حيث يتم تجاوز StartCommand لإرجاع "START".ثم يمكنك فقط التسجيل أو اكتشاف الفصول الدراسية.

إجراء 1+ على اقتراح الانعكاس، سيمنحك بنية أكثر عقلانية في صفك.

في الواقع ، يمكنك القيام بما يلي (إذا لم تفكر في الأمر بالفعل) إنشاء طرق مقابلة للسلسلة التي تتوقعها كوسيطة لطريقة مصنع getCommand () ، فكل ما عليك فعله هو التفكير والاستدعاء ( ) هذه الطرق وإرجاع الكائن الصحيح.

مرخصة بموجب: CC-BY-SA مع الإسناد
لا تنتمي إلى StackOverflow
scroll top