ISlugSupported interface should contain the slug entity name

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.
9 лет назад
Hi Andrei,

We are working on a new plugin that has an entity that is called "Category".
We have mapped this entity to the database to be in a table with a prefix i.e SS_Category and all is good.
But know we want to be able to have ID less urls for accessing these categories, so we are using the UlrRecordService to add slug url records for this entity. But the implementation of the UrlRecordService uses the type name, which is "Category". Since there is already slug url records for the categories in nopCommerce we have no other choice but to rename our entity to be with some prefix i.e SS_Category but this makes the code very ugly and the code maintenance too.
By looking at the options we have we think it would be best if the ISlugSupported interface, which is now empty, had a property called EntityName or SlugEnittyName or whatever. This way each entity can specify its UrlRecord entity name rather than it is taken form the type name.

public interface ISlugSupported
    {
        string EntityName { get; }
    }



It would be very convenient for all plugin vendors to be able to specify the names of their entities without conflicting with the already existing ones or the entities of other plugin vendors.

Hope this makes sense!
9 лет назад
Hi,

Thanks for suggestion. I agree that it could convenient for some vendors. But having this property just for such cases (when you have entities with names such as already "reserved" system entities) is a bit redundant. This property will make source less readable.

So you'll have to use some other class name (not "Category"). Furthermore, it's a bad practice to have two entities with the same name in distinct namespaces.
9 лет назад
a.m. wrote:
Hi,

Thanks for suggestion. I agree that it could convenient for some vendors. But having this property just for such cases (when you have entities with names such as already "reserved" system entities) is a bit redundant. This property will make source less readable.

So you'll have to use some other class name (not "Category"). Furthermore, it's a bad practice to have two entities with the same name in distinct namespaces.


Hi Andrei,

Of course we can rename it and we would have just done it without even bothering to write to you about it.
So my point was not to change this for the sake of changing it but to let the developers some freedom.
Why the core entity name should be "reserved"?
Now we have unnecessary restrictions to how to name our entities, which I can't call a good practice.
I think the plugin developers should be able to call their classes whatever they want to (actually if they have to follow the best practices they should name their entities with meaningful names and not SS_Category), without being restricted and without worrying that some other vendors might have decided to give their entity the same name and could break their plugins.

In the SaveSlug<T>(T entity, ...) where T : BaseEntity, ISlugSupported method the logic is like this:

string entityName = typeof(T).Name;
...
var urlRecord = new UrlRecord()
                    {
...
                        EntityName = entityName,
                      ...
                    };



For me this is an unnecessary restriction to name my types the same way I want them persisted in the database.
This is also not a good practice. This coupling of the names of the classes with how they are persisted to the database is a bad thing.

It would be better to have something like this:

string entityName = entity.EntityName;


And let each entity that is ISlugSupported to specify how it should be persisted in the UrlRecord table.
I don't think the code will get more complex as currently we have an empty ISlugSupported interface used only as a generic constraint but it is not used for anything else. Some people might say that the current code implementation is more complex as it requires an interface that is used nowehere.

I agree that in some cases it is not good to have several classes with the same name in different namespaces but this is only if you plan to use them both somewhere in your code. Actually .NET Framework is full with classes with the same names but in different namespaces.  So I would say it is a matter of personal preferences. For example we don't plan to use our category entity together with the nop category in the plugin's code, so it is absolutely ok to name it just "Category".

Anyway I felt I was obliged to suggest it.

Thanks
6 лет назад
Hello Andrei-

It appears that this is now a problem for me.

The Foxnetsoft nop Articles plugin and the 7spikes Knowledgebase plugin both have identically named entities, and as a result, neither plugin can function if both are installed in the same nopComerce instance. This is very disappointing and frustrating and I hope you will consider this suggestion from nop-templates for a future release so that other users and developers do not have this same problem.

Thanks,
Steve
This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.