The main points on your code that can be improved are described in the following sections. I can't really find anything that can improve the performance of your code, so i will focus more on how you can shorten your code and make it more readable.
Checkboxes state
You chose to store all 4 permissions in an object which is just fine. The key of the object is the type
of the permission and the value is the corresponding flag indicating if the permission is on/off (enabled
). You haven't used the enabled
part in your checkboxes in order to reflect the current state. That's why although you have set all permissions initially to true
, the checkboxes are not checked.
You can use Object.entries(permission).map(([key, enabled])
instead and provide the enabled
as the checked
attribute of the checkbox element.
Use of eval
In general you should avoid using eval. You can see a pattern in your createPerms
, editPerms
etc methods. They all have the same syntax and the only thing changing is the type
of the permission. That makes me think that you can create one method (instead of 4) and pass the type of the permission as a parameter. Your method could look like this:
const oneToRuleThemAll = (type) => {
setPermission({ ...permission, [type]: !permission[type] });
};
Naming
Naming is really important and helps other people reading your code (or even your future self) to quickly understand what your code and variables are intended to do. Some key points that can improve readability are:
- rename
permission
state to permissions
since it keeps all of them
- when iterating through the keys of your
permission
object you can use a more indicative name for each key, that states what this key represents. In your case type
(or even permissionType
) is much more explanatory than the generic key
name.
- the method's name
createPerms
, editPerms
etc are not really useful because they do not describe what each method does. These methods toggle the permission of the corresponding type (e.g. createPerms
toggles the value of create
permission). So a more valid name would be toggleCreatePermission
.
(But remember you don't have to do this since we will get rid of these 4 methods and just create one accepting type
as an argument)
Final code
const [permissions, setPermissions] = useState({
create: true,
view: true,
edit: true,
delete: true
});
const togglePermission = (type) => {
setPermissions({ ...permissions, [type]: !permissions[type] });
};
return (
<>
<h1>
<center> User Permission </center>
</h1>
<table className="ui celled table">
<thead>
<tr>
{Object.keys(permissions).map((type) => (
<th key={type}>{type}</th>
))}
</tr>
</thead>
<tbody>
<tr>
{Object.entries(permissions).map(([type, enabled]) => (
<td key={type}>
<input
type="checkbox"
onChange={() => togglePermission(type)}
checked={enabled}
/>
</td>
))}
</tr>
</tbody>
</table>
{permissions.create && <h3>Create</h3>}
{permissions.view && <h3>View</h3>}
{permissions.edit && <h3>Edit</h3>}
{permissions.delete && <h3>Delete</h3>}
</>
);
Extra stuff
The enhancement described here is an overkill for you component currently, but i added it to demonstrate how you can make your code even more extensible in the future and for educational purposes
So another improvement that can possibly make your code more easily extensible would be to store your permission types in a constant object and create them dynamically. Let me elaborate on that.
Let's suppose you want to add one more permission to your component and let's name this new permission authorize
. With the code above you need to add this new permission in your initial permissions
state like authorize: true
and add a new rendering section at the end to show if this is enabled or not like {permissions.authorize && <h3>Authorize</h3>}
.
You can create a static object that would look like this:
const PERMISSIONS = {
create: { name: 'Create', initial: true },
view: { name: 'View', initial: true },
edit: { name: 'Edit', initial: true },
delete: { name: 'Delete', initial: true }
}
and then create your initial state dynamically once like below:
const INITIAL_STATE = Object.entries(PERMISSIONS).reduce((result, [type, permission]) => {
result[type] = permission.initial;
return result;
}, {})
and initialize your state: useState(INITIAL_STATE)
Then you can also change the permissions rendering section to make it fully dynamic and make it like below:
Object.entries(permissions)
.filter(([type, enabled]) => enabled)
.map(([type]) => <h3>PERMISSIONS[type].name</h3>)
Having done all that you can add/remove permissions by just adding a new entry on your static PERMISSIONS
object. For the new authorize
example this would be:
const PERMISSIONS = {
create: { name: 'Create', initial: true },
view: { name: 'View', initial: true },
edit: { name: 'Edit', initial: true },
delete: { name: 'Delete', initial: true },
authorize: { name: 'Authorize', initial: false }
}